[rt #76782] improve documentation and error reporting for missing I/O callbacks

diff --git a/Changes b/Changes
index 24d2d2f718a0746803988fd444e53a650725881f..b05a9d598b32c2f9f20950a13e2c5a567780c97f 100644 (file)
--- a/Changes
+++ b/Changes
@@ -28,6 +28,18 @@ Bug fixes:
 
  - odd length RLE4 runs in BMP images were decoded incorrectly.
 
 
  - odd length RLE4 runs in BMP images were decoded incorrectly.
 

+Other changes:
+
+ - when reading or writing images via callbacks, the default callbacks
+   now push an error message indicating that a required callback was
+   called but not supplied.
+   https://rt.cpan.org/Ticket/Display.html?id=76782
+
+ - clarify which callbacks are required for reading and writing TIFF
+   images.
+
+ - improve logging for creation of callback I/O layers.
+

 Imager 0.89 - 18 Mar 2012
 ===========
 
 Imager 0.89 - 18 Mar 2012
 ===========
 

diff --git a/Imager.xs b/Imager.xs
index c869f552f4faa25b653c921cc2d2c5c899998d7e..171a738520aa878dbfc80298f0fc59ab5ec56e29 100644 (file)
--- a/Imager.xs
+++ b/Imager.xs
@@ -181,8 +181,11 @@ call_reader(struct cbdata *cbd, void *buf, size_t size,
   SV *data;
   dSP;
 
   SV *data;
   dSP;
 

-  if (!SvOK(cbd->readcb))
+  if (!SvOK(cbd->readcb)) {
+    mm_log((1, "read callback called but no readcb supplied\n"));
+    i_push_error(0, "read callback called but no readcb supplied");

     return -1;
     return -1;

+  }

 
   ENTER;
   SAVETMPS;
 
   ENTER;
   SAVETMPS;


@@ -230,8 +233,11 @@ io_seeker(void *p, off_t offset, int whence) {
   off_t result;
   dSP;
 
   off_t result;
   dSP;
 

-  if (!SvOK(cbd->seekcb))
+  if (!SvOK(cbd->seekcb)) {
+    mm_log((1, "seek callback called but no seekcb supplied\n"));
+    i_push_error(0, "seek callback called but no seekcb supplied");

     return -1;
     return -1;

+  }

 
   ENTER;
   SAVETMPS;
 
   ENTER;
   SAVETMPS;


@@ -266,8 +272,11 @@ io_writer(void *p, void const *data, size_t size) {
   dSP;
   bool success;
 
   dSP;
   bool success;
 

-  if (!SvOK(cbd->writecb))
+  if (!SvOK(cbd->writecb)) {
+    mm_log((1, "write callback called but no writecb supplied\n"));
+    i_push_error(0, "write callback called but no writecb supplied");

     return -1;
     return -1;

+  }

 
   ENTER;
   SAVETMPS;
 
   ENTER;
   SAVETMPS;


@@ -351,6 +360,27 @@ do_io_new_buffer(pTHX_ SV *data_sv) {
   return io_new_buffer(data, length, my_SvREFCNT_dec, data_sv);
 }
 
   return io_new_buffer(data, length, my_SvREFCNT_dec, data_sv);
 }
 

+static const char *
+describe_sv(SV *sv) {
+  if (SvOK(sv)) {
+    if (SvROK(sv)) {
+      svtype type = SvTYPE(SvRV(sv));
+      switch (type) {
+      case SVt_PVCV: return "CV";
+      case SVt_PVGV: return "GV";
+      case SVt_PVLV: return "LV";
+      default: return "some reference";
+      }
+    }
+    else {
+      return "non-reference scalar";
+    }
+  }
+  else {
+    return "undef";
+  }
+}
+

 static i_io_glue_t *
 do_io_new_cb(pTHX_ SV *writecb, SV *readcb, SV *seekcb, SV *closecb) {
   struct cbdata *cbd;
 static i_io_glue_t *
 do_io_new_cb(pTHX_ SV *writecb, SV *readcb, SV *seekcb, SV *closecb) {
   struct cbdata *cbd;


@@ -361,6 +391,8 @@ do_io_new_cb(pTHX_ SV *writecb, SV *readcb, SV *seekcb, SV *closecb) {
   cbd->seekcb = newSVsv(seekcb);
   cbd->closecb = newSVsv(closecb);
 
   cbd->seekcb = newSVsv(seekcb);
   cbd->closecb = newSVsv(closecb);
 

+  mm_log((1, "do_io_new_cb(writecb %p (%s), readcb %p (%s), seekcb %p (%s), closecb %p (%s))\n", writecb, describe_sv(writecb), readcb, describe_sv(readcb), seekcb, describe_sv(seekcb), closecb, describe_sv(closecb)));
+

   return io_new_cb(cbd, io_reader, io_writer, io_seeker, io_closer, 
                   io_destroyer);
 }
   return io_new_cb(cbd, io_reader, io_writer, io_seeker, io_closer, 
                   io_destroyer);
 }

diff --git a/lib/Imager/Files.pod b/lib/Imager/Files.pod
index 916fcf65b126ecbe9e0c8f5d1a78f28861ec244c..9e2f855cdad180dbdccca045dc580ea8e5783065 100644 (file)
--- a/lib/Imager/Files.pod
+++ b/lib/Imager/Files.pod
@@ -273,12 +273,31 @@ When reading from a file you can use either C<callback> or C<readcb>
 to supply the read callback, and when writing C<callback> or
 C<writecb> to supply the write callback.
 
 to supply the read callback, and when writing C<callback> or
 C<writecb> to supply the write callback.
 

-Some file formats, currently only C<TIFF>, also require a C<seekcb>
-parameter to change position in the file.  If no C<seekcb> parameter
-is provided a default will be provided that fails.
+Whether reading or writing a C<TIFF> image, C<seekcb> and C<readcb>
+are required.
+
+If a file handler attempts to use C<readcb>, C<writecb> or C<seekcb>
+and you haven't supplied one, the call will fail, failing the image
+read or write, returning an error message indicating that the callback
+is missing:
+
+  # attempting to read a TIFF image without a seekcb
+  open my $fh, "<", $filename or die;
+  my $rcb = sub {
+    my $val;
+    read($fh, $val, $_[0]) or return "";
+    return $val;
+  };
+  my $im = Imager->new(callback => $rcb)
+    or die Imager->errstr
+  # dies with (wrapped here):
+  # Error opening file: (Iolayer): Failed to read directory at offset 0:
+  # (Iolayer): Seek error accessing TIFF directory: seek callback called
+  # but no seekcb supplied

 
 You can also provide a C<closecb> parameter called when writing the
 
 You can also provide a C<closecb> parameter called when writing the

-file is complete. 
+file is complete.  If no C<closecb> is supplied the default will
+succeed silently.

 
   # contrived
   my $data;
 
   # contrived
   my $data;


@@ -1068,6 +1087,15 @@ some page other than the first.  The page is 0 based:
 If you read an image with multiple alpha channels, then only the first
 alpha channel will be read.
 
 If you read an image with multiple alpha channels, then only the first
 alpha channel will be read.
 

+When reading a C<TIFF> image with callbacks, the C<seekcb> callback
+parameter is also required.
+
+When writing a C<TIFF> image with callbacks, the C<seekcb> and
+C<readcb> parameters are also required.
+
+C<TIFF> is a random access file format, it cannot be read from or
+written to unseekable streams such as pipes or sockets.
+

 =head2 BMP (Windows Bitmap)
 
 Imager can write 24-bit RGB, and 8, 4 and 1-bit per pixel paletted
 =head2 BMP (Windows Bitmap)
 
 Imager can write 24-bit RGB, and 8, 4 and 1-bit per pixel paletted

diff --git a/t/t07iolayer.t b/t/t07iolayer.t
index bead870be7434b19a08d1888f1df8350efc7af44..447489734865f0da0f05c5d84770fa59279f62a8 100644 (file)
--- a/t/t07iolayer.t
+++ b/t/t07iolayer.t
@@ -1,6 +1,6 @@
 #!perl -w
 use strict;
 #!perl -w
 use strict;

-use Test::More tests => 246;
+use Test::More tests => 252;

 # for SEEK_SET etc, Fcntl doesn't provide these in 5.005_03
 use IO::Seekable;
 
 # for SEEK_SET etc, Fcntl doesn't provide these in 5.005_03
 use IO::Seekable;
 


@@ -808,6 +808,32 @@ SKIP:
   }
 }
 
   }
 }
 

+{ # based on discussion on IRC, user was attempting to write a TIFF
+  # image file with only a write callback, but TIFF requires seek and
+  # read callbacks when writing.
+  # https://rt.cpan.org/Ticket/Display.html?id=76782
+  my $cb = Imager::io_new_cb(undef, undef, undef, undef);
+  {
+    Imager::i_clear_error();
+    my $data;
+    is($cb->read($data, 10), undef, "default read callback should fail");
+    is(Imager->_error_as_msg(), "read callback called but no readcb supplied",
+       "check error message");
+  }
+  {
+    Imager::i_clear_error();
+    is($cb->raw_write("abc"), -1, "default write callback should fail");
+    is(Imager->_error_as_msg(), "write callback called but no writecb supplied",
+       "check error message");
+  }
+  {
+    Imager::i_clear_error();
+    is($cb->seek(0, 0), -1, "default seek callback should fail");
+    is(Imager->_error_as_msg(), "seek callback called but no seekcb supplied",
+       "check error message");
+  }
+}
+

 Imager->close_log;
 
 unless ($ENV{IMAGER_KEEP_FILES}) {
 Imager->close_log;
 
 unless ($ENV{IMAGER_KEEP_FILES}) {

diff --git a/t/x20spell.t b/t/x20spell.t
index 4b97643b4bc5fdf24d37eb43c7659b8d57a0e6c5..74e4e46b561b9230571d00a02252c78eba953f7f 100644 (file)
--- a/t/x20spell.t
+++ b/t/x20spell.t
@@ -52,6 +52,7 @@ preloads
 renderer
 tuple
 unary
 renderer
 tuple
 unary

+unseekable

 varargs
 /;
 
 varargs
 /;