- error messages when writing TIFF images were always

  'Could not write to buffer', more useful messages are now reported.
- error messages when writing PNM images were always
  'unable to write pnm image', more useful messages are now reported.

diff --git a/Changes b/Changes
index 14dce3bacec6d1deac71df20535c9ef7cb0a3910..a8813ef0a56c4f087906b29cc33e72eac43fdc2f 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1302,6 +1302,10 @@ Revision history for Perl extension Imager.
   Thanks to David Wheeler for his help in tracking this down.
 - reword and provide an example for non-proportionally scaling an
   image.  Wording from Simon Cozens.
+- error messages when writing TIFF images were always 
+  'Could not write to buffer', more useful messages are now reported.
+- error messages when writing PNM images were always
+  'unable to write pnm image', more useful messages are now reported.
 
 =================================================================
 

diff --git a/Imager.pm b/Imager.pm
index fe0a874d2f617203b8b54a6554187ad0b09f6263..244d14cd6ce0f1cd4fb67e3c7209f407a5a6c032 100644 (file)
--- a/Imager.pm
+++ b/Imager.pm
@@ -1217,7 +1217,8 @@ sub read {
   if ( $input{'type'} eq 'pnm' ) {
     $self->{IMG}=i_readpnm_wiol( $IO, -1 ); # Fixme, check if that length parameter is ever needed
     if ( !defined($self->{IMG}) ) {
-      $self->{ERRSTR}='unable to read pnm image: '._error_as_msg(); return undef;
+      $self->{ERRSTR}='unable to read pnm image: '._error_as_msg(); 
+      return undef;
     }
     $self->{DEBUG} && print "loading a pnm file\n";
     return $self;
@@ -1446,12 +1447,12 @@ sub write {
 
     if (defined $input{class} && $input{class} eq 'fax') {
       if (!i_writetiff_wiol_faxable($self->{IMG}, $IO, $input{fax_fine})) {
-       $self->{ERRSTR}='Could not write to buffer';
+       $self->{ERRSTR} = $self->_error_as_msg();
        return undef;
       }
     } else {
       if (!i_writetiff_wiol($self->{IMG}, $IO)) {
-       $self->{ERRSTR}='Could not write to buffer';
+       $self->{ERRSTR} = $self->_error_as_msg();
        return undef;
       }
     }
@@ -1459,7 +1460,7 @@ sub write {
     $self->_set_opts(\%input, "pnm_", $self)
       or return undef;
     if ( ! i_writeppm_wiol($self->{IMG},$IO) ) {
-      $self->{ERRSTR}='unable to write pnm image';
+      $self->{ERRSTR} = $self->_error_as_msg();
       return undef;
     }
     $self->{DEBUG} && print "writing a pnm file\n";

diff --git a/iolayer.c b/iolayer.c
index 7d500cf721de6aa5d7a0db263d0c04625a108206..1bbda1158300e4818a4c179b781d3fbeaf220b1f 100644 (file)
--- a/iolayer.c
+++ b/iolayer.c
@@ -7,6 +7,7 @@
 #include <io.h>
 #endif
 #include <string.h>
+#include <errno.h>
 
 #define IOL_DEB(x)
 
@@ -1071,19 +1072,33 @@ static ssize_t fd_read(io_glue *ig, void *buf, size_t count) {
 }
 
 static ssize_t fd_write(io_glue *ig, const void *buf, size_t count) {
+  ssize_t result;
 #ifdef _MSC_VER
-  return _write(ig->source.fdseek.fd, buf, count);
+  result = _write(ig->source.fdseek.fd, buf, count);
 #else
-  return write(ig->source.fdseek.fd, buf, count);
+  result = write(ig->source.fdseek.fd, buf, count);
 #endif
+
+  if (result <= 0) {
+    i_push_errorf(errno, "write() failure: %s (%d)", strerror(errno), errno);
+  }
+
+  return result;
 }
 
 static off_t fd_seek(io_glue *ig, off_t offset, int whence) {
+  off_t result;
 #ifdef _MSC_VER
-  return _lseek(ig->source.fdseek.fd, offset, whence);
+  result = _lseek(ig->source.fdseek.fd, offset, whence);
 #else
-  return lseek(ig->source.fdseek.fd, offset, whence);
+  result = lseek(ig->source.fdseek.fd, offset, whence);
 #endif
+
+  if (result == (off_t)-1) {
+    i_push_errorf(errno, "lseek() failure: %s (%d)", strerror(errno), errno);
+  }
+
+  return result;
 }
 
 static void fd_close(io_glue *ig) {

diff --git a/t/t104ppm.t b/t/t104ppm.t
index da4eb38f22893899c2c204423a007a928c6ecf57..fa942cc5ecfb524c11b88ef501956f643b7768a9 100644 (file)
--- a/t/t104ppm.t
+++ b/t/t104ppm.t
@@ -1,7 +1,7 @@
 #!perl -w
 use Imager ':all';
 use lib 't';
-use Test::More tests => 60;
+use Test::More tests => 64;
 use strict;
 
 init_log("testout/t104ppm.log",1);
@@ -195,6 +195,18 @@ check_gray(Imager::i_get_pixel($ooim->{IMG}, 1, 1), 255);
   Imager->set_file_limits(reset=>1);
 }
 
+{ # check error messages set correctly
+  my $im = Imager->new(xsize=>100, ysize=>100, channels=>4);
+  ok(!$im->write(file=>"testout/t104_fail.ppm", type=>'pnm'),
+     "should fail to write 4 channel image");
+  is($im->errstr, 'can only save 1 or 3 channel images to pnm',
+     "check error message");
+  ok(!$im->read(file=>'t/t104ppm.t', type=>'pnm'),
+     'should fail to read script as an image file');
+  is($im->errstr, 'unable to read pnm image: bad header magic, not a PNM file',
+     "check error message");
+}
+
 sub openimage {
   my $fname = shift;
   local(*FH);

diff --git a/t/t106tiff.t b/t/t106tiff.t
index e58b5d51e323a412710a35c982f086dca68d60f6..18e296408d6cf5df0cb5b0ec2741dea2647a765d 100644 (file)
--- a/t/t106tiff.t
+++ b/t/t106tiff.t
@@ -1,7 +1,7 @@
 #!perl -w
 use strict;
 use lib 't';
-use Test::More tests => 89;
+use Test::More tests => 101;
 use Imager qw(:all);
 $^W=1; # warnings during command-line tests
 $|=1;  # give us some progress in the test harness
@@ -32,7 +32,7 @@ SKIP:
     $im = Imager->new(xsize=>2, ysize=>2);
     ok(!$im->write(file=>"testout/notiff.tif"), "should fail to write tiff");
     is($im->errstr, 'format not supported', "check no tiff message");
-    skip("no tiff support", 85);
+    skip("no tiff support", 97);
   }
 
   Imager::i_tags_add($img, "i_xres", 0, "300", 0);
@@ -366,4 +366,45 @@ SKIP:
        "check out of range page");
     is($imwork->errstr, "could not switch to page 2", "check message");
   }
+
+  { # test writing returns an error message correctly
+    # open a file read only and try to write to it
+    open TIFF, "> testout/t106_empty.tif" or die;
+    close TIFF;
+    open TIFF, "< testout/t106_empty.tif"
+      or skip "Cannot open testout/t106_empty.tif for reading", 8;
+    binmode TIFF;
+    my $im = Imager->new(xsize=>100, ysize=>100);
+    ok(!$im->write(fh => \*TIFF, type=>'tiff'),
+       "fail to write to read only handle");
+    cmp_ok($im->errstr, '=~', 'Could not create TIFF object: Error writing TIFF header: write\(\)',
+          "check error message");
+    ok(!Imager->write_multi({ type => 'tiff', fh => \*TIFF }, $im),
+       "fail to write multi to read only handle");
+    cmp_ok(Imager->errstr, '=~', 'Could not create TIFF object: Error writing TIFF header: write\(\)',
+          "check error message");
+    ok(!$im->write(fh => \*TIFF, type=>'tiff', class=>'fax'),
+       "fail to write to read only handle (fax)");
+    cmp_ok($im->errstr, '=~', 'Could not create TIFF object: Error writing TIFF header: write\(\)',
+          "check error message");
+    ok(!Imager->write_multi({ type => 'tiff', fh => \*TIFF, class=>'fax' }, $im),
+       "fail to write multi to read only handle (fax)");
+    cmp_ok(Imager->errstr, '=~', 'Could not create TIFF object: Error writing TIFF header: write\(\)',
+          "check error message");
+  }
+
+  { # test reading returns an error correctly - use test script as an
+    # invalid TIFF file
+    my $im = Imager->new;
+    ok(!$im->read(file=>'t/t106tiff.t', type=>'tiff'),
+       "fail to read script as image");
+    is($im->errstr, 
+       "Error opening file: Not a TIFF file, bad magic number 8483 (0x2123)", 
+       "check error message");
+    my @ims = Imager->read_multi(file =>'t/t106tiff.t', type=>'tiff');
+    ok(!@ims, "fail to read_multi script as image");
+    is($im->errstr, 
+       "Error opening file: Not a TIFF file, bad magic number 8483 (0x2123)", 
+       "check error message");
+  }
 }

diff --git a/tiff.c b/tiff.c
index 4f2b0e6e97ced44f2ee887e8de6fa42b219d8104..2ac76674f55a80f00cc8280b4706a2fd5afec1d8 100644 (file)
--- a/tiff.c
+++ b/tiff.c
@@ -431,7 +431,7 @@ i_readtiff_wiol(io_glue *ig, int length, int page) {
   
   if (!tif) {
     mm_log((1, "i_readtiff_wiol: Unable to open tif file\n"));
-    i_push_error(0, "opening file");
+    i_push_error(0, "Error opening file");
     TIFFSetErrorHandler(old_handler);
     TIFFSetWarningHandler(old_warn_handler);
     return NULL;
@@ -497,7 +497,7 @@ i_readtiff_multi_wiol(io_glue *ig, int length, int *count) {
   
   if (!tif) {
     mm_log((1, "i_readtiff_wiol: Unable to open tif file\n"));
-    i_push_error(0, "opening file");
+    i_push_error(0, "Error opening file");
     TIFFSetErrorHandler(old_handler);
     TIFFSetWarningHandler(old_warn_handler);
     return NULL;
@@ -899,8 +899,11 @@ Stores an image in the iolayer object.
 undef_int
 i_writetiff_multi_wiol(io_glue *ig, i_img **imgs, int count) {
   TIFF* tif;
+  TIFFErrorHandler old_handler;
   int i;
 
+  old_handler = TIFFSetErrorHandler(error_handler);
+
   io_glue_commit_types(ig);
   i_clear_error();
   mm_log((1, "i_writetiff_multi_wiol(ig 0x%p, imgs 0x%p, count %d)\n", 
@@ -922,24 +925,30 @@ i_writetiff_multi_wiol(io_glue *ig, i_img **imgs, int count) {
 
 
   if (!tif) {
-    mm_log((1, "i_writetiff_mulit_wiol: Unable to open tif file for writing\n"));
+    mm_log((1, "i_writetiff_multi_wiol: Unable to open tif file for writing\n"));
+    i_push_error(0, "Could not create TIFF object");
+    TIFFSetErrorHandler(old_handler);
     return 0;
   }
 
   for (i = 0; i < count; ++i) {
     if (!i_writetiff_low(tif, imgs[i])) {
       TIFFClose(tif);
+      TIFFSetErrorHandler(old_handler);
       return 0;
     }
 
     if (!TIFFWriteDirectory(tif)) {
       i_push_error(0, "Cannot write TIFF directory");
       TIFFClose(tif);
+      TIFFSetErrorHandler(old_handler);
       return 0;
     }
   }
 
+  TIFFSetErrorHandler(old_handler);
   (void) TIFFClose(tif);
+
   return 1;
 }
 
@@ -960,6 +969,9 @@ undef_int
 i_writetiff_multi_wiol_faxable(io_glue *ig, i_img **imgs, int count, int fine) {
   TIFF* tif;
   int i;
+  TIFFErrorHandler old_handler;
+
+  old_handler = TIFFSetErrorHandler(error_handler);
 
   io_glue_commit_types(ig);
   i_clear_error();
@@ -983,23 +995,29 @@ i_writetiff_multi_wiol_faxable(io_glue *ig, i_img **imgs, int count, int fine) {
 
   if (!tif) {
     mm_log((1, "i_writetiff_mulit_wiol: Unable to open tif file for writing\n"));
+    i_push_error(0, "Could not create TIFF object");
+    TIFFSetErrorHandler(old_handler);
     return 0;
   }
 
   for (i = 0; i < count; ++i) {
     if (!i_writetiff_low_faxable(tif, imgs[i], fine)) {
       TIFFClose(tif);
+      TIFFSetErrorHandler(old_handler);
       return 0;
     }
 
     if (!TIFFWriteDirectory(tif)) {
       i_push_error(0, "Cannot write TIFF directory");
       TIFFClose(tif);
+      TIFFSetErrorHandler(old_handler);
       return 0;
     }
   }
 
   (void) TIFFClose(tif);
+  TIFFSetErrorHandler(old_handler);
+
   return 1;
 }
 
@@ -1016,13 +1034,16 @@ Stores an image in the iolayer object.
 undef_int
 i_writetiff_wiol(i_img *img, io_glue *ig) {
   TIFF* tif;
+  TIFFErrorHandler old_handler;
+
+  old_handler = TIFFSetErrorHandler(error_handler);
 
   io_glue_commit_types(ig);
   i_clear_error();
   mm_log((1, "i_writetiff_wiol(img %p, ig 0x%p)\n", img, ig));
 
   /* FIXME: Enable the mmap interface */
-  
+
   tif = TIFFClientOpen("No name", 
                       "wm",
                       (thandle_t) ig, 
@@ -1038,15 +1059,20 @@ i_writetiff_wiol(i_img *img, io_glue *ig) {
 
   if (!tif) {
     mm_log((1, "i_writetiff_wiol: Unable to open tif file for writing\n"));
+    i_push_error(0, "Could not create TIFF object");
+    TIFFSetErrorHandler(old_handler);
     return 0;
   }
 
   if (!i_writetiff_low(tif, img)) {
     TIFFClose(tif);
+    TIFFSetErrorHandler(old_handler);
     return 0;
   }
 
   (void) TIFFClose(tif);
+  TIFFSetErrorHandler(old_handler);
+
   return 1;
 }
 
@@ -1070,6 +1096,9 @@ point.
 undef_int
 i_writetiff_wiol_faxable(i_img *im, io_glue *ig, int fine) {
   TIFF* tif;
+  TIFFErrorHandler old_handler;
+
+  old_handler = TIFFSetErrorHandler(error_handler);
 
   io_glue_commit_types(ig);
   i_clear_error();
@@ -1092,15 +1121,20 @@ i_writetiff_wiol_faxable(i_img *im, io_glue *ig, int fine) {
 
   if (!tif) {
     mm_log((1, "i_writetiff_wiol: Unable to open tif file for writing\n"));
+    i_push_error(0, "Could not create TIFF object");
+    TIFFSetErrorHandler(old_handler);
     return 0;
   }
 
   if (!i_writetiff_low_faxable(tif, im, fine)) {
     TIFFClose(tif);
+    TIFFSetErrorHandler(old_handler);
     return 0;
   }
 
   (void) TIFFClose(tif);
+  TIFFSetErrorHandler(old_handler);
+
   return 1;
 }