From 2691d2205c1bfbebe0fc4c47534c89ccb4e62e08 Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Wed, 18 Jan 2006 12:18:51 +0000 Subject: [PATCH] - 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. --- Changes | 4 ++++ Imager.pm | 9 +++++---- iolayer.c | 23 +++++++++++++++++++---- t/t104ppm.t | 14 +++++++++++++- t/t106tiff.t | 45 +++++++++++++++++++++++++++++++++++++++++++-- tiff.c | 42 ++++++++++++++++++++++++++++++++++++++---- 6 files changed, 122 insertions(+), 15 deletions(-) diff --git a/Changes b/Changes index 14dce3ba..a8813ef0 100644 --- 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 fe0a874d..244d14cd 100644 --- 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 7d500cf7..1bbda115 100644 --- a/iolayer.c +++ b/iolayer.c @@ -7,6 +7,7 @@ #include #endif #include +#include #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 da4eb38f..fa942cc5 100644 --- 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 e58b5d51..18e29640 100644 --- 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 4f2b0e6e..2ac76674 100644 --- 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; } -- 2.30.2