From: Tony Cook <tony@develop-help.com>
Date: Thu, 22 Sep 2011 12:36:01 +0000 (+1000)
Subject: WIP, more coverage and fixes
X-Git-Tag: v_iobuf~15
X-Git-Url: http://git.imager.perl.org/imager.git/commitdiff_plain/28a5ceb962c54f01d68115c8ce23cf47c9c33742

WIP, more coverage and fixes
---

diff --git a/iolayer.c b/iolayer.c
index e4513954..9318778b 100644
--- a/iolayer.c
+++ b/iolayer.c
@@ -1198,7 +1198,9 @@ i_io_read_fill(io_glue *ig, ssize_t needed) {
   ssize_t rc;
   int good = 0;
 
-  /* this condition may be unused, callers should also be checking it */
+  IOL_DEB(fprintf(IOL_DEBs, "i_io_read_fill(%p, %d)\n", ig, (int)needed));
+
+  /* these conditions may be unused, callers should also be checking them */
   if (ig->error || ig->buf_eof)
     return 0;
 
@@ -1233,16 +1235,23 @@ i_io_read_fill(io_glue *ig, ssize_t needed) {
     needed -= rc;
   }
 
-  if (rc < 0)
+  if (rc < 0) {
     ig->error = 1;
-  else if (rc == 0)
+    IOL_DEB(fprintf(IOL_DEBs, " i_io_read_fill -> rc %d, setting error\n",
+		    (int)rc));
+  }
+  else if (rc == 0) {
     ig->buf_eof = 1;
+    IOL_DEB(fprintf(IOL_DEBs, " i_io_read_fill -> rc 0, setting eof\n"));
+  }
 
   if (good) {
     ig->read_ptr = buf_start;
     ig->read_end = work;
   }
   
+  IOL_DEB(fprintf(IOL_DEBs, "i_io_read_fill => %d, %u buffered\n", good,
+		  (unsigned)(ig->read_end - ig->read_ptr)));
   return good;
 }
 
@@ -1353,6 +1362,12 @@ ssize_t
 i_io_peekn(io_glue *ig, void *buf, size_t size) {
   IOL_DEB(fprintf(IOL_DEBs, "i_io_peekn(%p, %p, %d)\n", ig, buf, (int)size));
 
+  if (size == 0) {
+    i_push_error(0, "peekn size must be positive");
+    IOL_DEB(fprintf(IOL_DEBs, "i_io_peekn() => -1 (zero size)\n"));
+    return -1;
+  }
+
   if (ig->write_ptr) {
     IOL_DEB(fprintf(IOL_DEBs, "i_io_peekn() => -1 (write_ptr set)\n"));
     return -1;
@@ -1361,28 +1376,28 @@ i_io_peekn(io_glue *ig, void *buf, size_t size) {
   if (!ig->buffer)
     i_io_setup_buffer(ig);
 
-  if (!ig->read_ptr || size > ig->read_end - ig->read_ptr) {
-    if (ig->error) {
-      IOL_DEB(fprintf(IOL_DEBs, "i_io_peekn() => -1 (error set)\n"));
-      return -1;
-    }
-    if (ig->buf_eof) {
-      IOL_DEB(fprintf(IOL_DEBs, "i_io_peekn() => 0 (eof set)\n"));
-      return 0;
-    }
-
-    if (!i_io_read_fill(ig, size)) {
-      ssize_t result = ig->error ? -1 : 0;
-      IOL_DEB(fprintf(IOL_DEBs, "i_io_peekn() => %d (read fill failed)\n",
-		      (int)result));
-      return result;
-    }
+  if ((!ig->read_ptr || size > ig->read_end - ig->read_ptr)
+      && !(ig->buf_eof || ig->error)) {
+    i_io_read_fill(ig, size);
   }
   
   if (size > ig->read_end - ig->read_ptr)
     size = ig->read_end - ig->read_ptr;
 
-  memcpy(buf, ig->read_ptr, size);
+  if (size)
+    memcpy(buf, ig->read_ptr, size);
+  else if (ig->buf_eof) {
+    IOL_DEB(fprintf(IOL_DEBs, "i_io_peekn() => 0 (eof)\n"));
+    return 0;
+  }
+  else if (ig->error) {
+    IOL_DEB(fprintf(IOL_DEBs, "i_io_peekn() => -1 (error)\n"));
+    return -1;
+  }
+  else {
+    IOL_DEB(fprintf(IOL_DEBs, "i_io_peekn() - size 0 but not eof or error!\n"));
+    return -1;
+  }
 
   IOL_DEB(fprintf(IOL_DEBs, "i_io_peekn() => %d\n", (int)size));
 
@@ -1395,8 +1410,18 @@ i_io_putc_imp(io_glue *ig, int c) {
 
   if (!ig->buffered) {
     char buf = c;
-    ssize_t write_result = i_io_raw_write(ig, &buf, 1);
-    int result = write_result == 1 ? c : EOF;
+    ssize_t write_result;
+
+    if (ig->error)
+      return EOF;
+
+    write_result = i_io_raw_write(ig, &buf, 1);
+    int result = c;
+    if (write_result != 1) {
+      ig->error = 1;
+      result = EOF;
+      IOL_DEB(fprintf(IOL_DEBs, "  unbuffered putc() failed, setting error mode\n"));
+    }
     IOL_DEB(fprintf(IOL_DEBs, "  unbuffered: result %d\n", result));
 
     return result;
diff --git a/t/t07iolayer.t b/t/t07iolayer.t
index 2d93f14b..75719108 100644
--- a/t/t07iolayer.t
+++ b/t/t07iolayer.t
@@ -1,6 +1,6 @@
 #!perl -w
 use strict;
-use Test::More tests => 159;
+use Test::More tests => 198;
 # for SEEK_SET etc, Fcntl doesn't provide these in 5.005_03
 use IO::Seekable;
 
@@ -504,7 +504,8 @@ SKIP:
     is($ops, "R8192>10;R8182>10;R8172>10;R8162>10;R8152>10;R8142>10;R8132>10;",
        "check we got the raw calls expected");
   }
-  { # peekn followed by errors
+  for my $buffered (1, 0) { # peekn followed by errors
+    my $buffered_desc = $buffered ? "buffered" : "unbuffered";
     my $read = 0;
     my $base = "abcdef";
     my $pos = 0;
@@ -527,18 +528,48 @@ SKIP:
       return $result;
     };
     my $io = Imager::io_new_cb(undef, $reader, undef, undef);
-    is($io->peekn(5), "abcde", "peekn until just before error");
-    is($io->peekn(6), "abcdef", "peekn until error");
-    is($io->peekn(7), "abcdef", "peekn past error");
-    ok(!$io->error, "should be no error indicator, since data buffered");
-    ok(!$io->eof, "should be no eof indicator, since data buffered");
+    ok($io, "make $buffered_desc cb with error after 6 bytes");
+    is($io->peekn(5), "abcde",
+       "peekn until just before error ($buffered_desc)");
+    is($io->peekn(6), "abcdef", "peekn until error ($buffered_desc)");
+    is($io->peekn(7), "abcdef", "peekn past error ($buffered_desc)");
+    ok(!$io->error,
+       "should be no error indicator, since data buffered ($buffered_desc)");
+    ok(!$io->eof,
+       "should be no eof indicator, since data buffered ($buffered_desc)");
 
     # consume it
-    is($io->read2(6), "abcdef", "consume the buffer");
-    is($io->peekn(10), undef, "should get an error indicator");
-    ok($io->error, "should be an error state");
+    is($io->read2(6), "abcdef", "consume the buffer ($buffered_desc)");
+    is($io->peekn(10), undef,
+       "peekn should get an error indicator ($buffered_desc)");
+    ok($io->error, "should be an error state ($buffered_desc)");
+    ok(!$io->eof, "but not eof ($buffered_desc)");
+  }
+  { # peekn on an empty file
+    my $io = Imager::io_new_buffer("");
+    is($io->peekn(10), "", "peekn on empty source");
+    ok($io->eof, "should be in eof state");
+    ok(!$io->error, "but not error");
+  }
+  { # peekn on error source
+    my $io = Imager::io_new_cb(undef, sub { return; }, undef, undef);
+    is($io->peekn(10), undef, "peekn on empty source");
+    ok($io->error, "should be in error state");
     ok(!$io->eof, "but not eof");
   }
+  { # peekn on short source
+    my $io = Imager::io_new_buffer("abcdef");
+    is($io->peekn(4), "abcd", "peekn 4 on 6 byte source");
+    is($io->peekn(10), "abcdef", "followed by peekn 10 on 6 byte source");
+    is($io->peekn(10), "abcdef", "and again, now eof is set");
+  }
+  { # peekn(0)
+    Imager::i_clear_error();
+    my $io = Imager::io_new_buffer("abcdef");
+    is($io->peekn(0), undef, "peekn 0 on 6 byte source");
+    my $msg = Imager->_error_as_msg;
+    is($msg, "peekn size must be positive");
+  }
   { # getc through a whole file (buffered)
     my $io = Imager::io_new_buffer($base);
     my $out = '';
@@ -622,6 +653,51 @@ SKIP:
     ok($io->error, "io marked error");
     ok(!$io->eof, "but not eof");
   }
+  { # initial putc
+    my $io = Imager::io_new_bufchain();
+    is($io->putc(ord "A"), ord "A", "initial putc buffered");
+    is($io->close, 0, "close it");
+    is(Imager::io_slurp($io), "A", "check it was written");
+  }
+  { # initial putc - unbuffered
+    my $io = Imager::io_new_bufchain();
+    $io->set_buffered(0);
+    is($io->putc(ord "A"), ord "A", "initial putc unbuffered");
+    is($io->close, 0, "close it");
+    is(Imager::io_slurp($io), "A", "check it was written");
+  }
+  { # putc unbuffered with error
+    my $io = Imager::io_new_cb(undef, undef, undef, undef);
+    $io->set_buffered(0);
+    is($io->putc(ord "A"), -1, "initial putc unbuffered error");
+    ok($io->error, "io in error");
+    is($io->putc(ord "B"), -1, "still in error");
+  }
+  { # putc while in read state
+    my $io = Imager::io_new_cb(sub { 1 }, sub { return "AA" }, undef, undef);
+    is($io->getc, ord "A", "read to setup read buffer");
+    is($io->putc(ord "B"), -1, "putc should fail");
+  }
+  { # buffered putc error handling
+    # tests the check for error state in the buffered putc code
+    my $io = Imager::io_new_cb(undef, undef, undef, undef);
+    $io->putc(ord "A");
+    ok(!$io->flush, "flush should fail");
+    ok($io->error, "should be in error state");
+    is($io->putc(ord "B"), -1, "check for error");
+  }
+  { # buffered putc flush error handling
+    # test handling of flush failure and of the error state resulting
+    # from that
+    my $io = Imager::io_new_cb(undef, undef, undef, undef);
+    my $i = 0;
+    while (++$i < 100_000 && $io->putc(ord "A") == ord "A") {
+      # until we have to flush and fail doing do
+    }
+    is($i, 8193, "should have failed on 8193rd byte");
+    ok($io->error, "should be in error state");
+    is($io->putc(ord "B"), -1, "next putc should fail");
+  }
 }
 
 Imager->close_log;