Bug #48350 for IO-Compress-Base:

Fri Jul 31 06:36:28 2009 PUBNOOP [...] cpan.org - Ticket created

IO::Compress::Base fails to trap read errors on the input filehandle. Tested against 0.020 from blead. Failing test attached. Output: 1..1 read() on closed filehandle STDIN at /home/dave/perlobj/installblead/lib/5.11.0/IO/Compress/Base.pm line 489. Use of uninitialized value $status in numeric gt (>) at /home/dave/perlobj/installblead/lib/5.11.0/IO/Compress/Base.pm line 489. Use of uninitialized value $status in numeric lt (<) at /home/dave/perlobj/installblead/lib/5.11.0/IO/Compress/Base.pm line 495. not ok 1 - bzip2 failed on input read failure # Failed test 'bzip2 failed on input read failure' # at t.pl line 12. # got: '1' # expected: '0' # Looks like you failed 1 test of 1.

Subject:

t.pl

use strict; use warnings; use Test::More tests => 1; use IO::Compress::Bzip2 qw/bzip2/; close STDIN; my $input = "0921783497324932749817234"; my $dest; my $result = bzip2 \*STDIN => \$dest; is $result, 0, "bzip2 failed on input read failure";

Fri Jul 31 18:38:03 2009 pmqs [...] cpan.org - Correspondence added

Hi Dave, thanks for the report. I've silenced the two numeric comparison warnings, but the "read() on closed filehandle" warning will be trickier/impractical to deal with. Real filehandles are fine - an undefined fileno is enough to tell if a handle is closed. The problem is tied filehandles - most make fileno return undef all the time, so that can't be used. Some, like IO-String, provide an "opened" method, which would be ideal for my purposes. The problem is there are tied filehandle implementations that don't have an opened method. Paul

Fri Jul 31 18:38:04 2009 The RT System itself - Status changed from 'new' to 'open'

Sat Aug 01 01:29:55 2009 PUBNOOP [...] cpan.org - Correspondence added

On Fri Jul 31 18:38:03 2009, PMQS wrote: Show quoted text

> Hi Dave, > > thanks for the report. I've silenced the two numeric comparison > warnings, but the "read() on closed filehandle" warning will be > trickier/impractical to deal with.

IMO you don't want to silence that - if some fool feeds a closed filehandle to IO::Compress::* as input then they should get warned about it, just as if they'd read from a closed filehandle themselves. I used a closed filehandle in the bug report as a quick way to cause a failing read() call. I didn't mean that you should be silencing that warning. To simulate a read() failure on an open filehandle you could: { package MockHandle; use base qw/IO::String/; use Errno qw/EIO/; sub READ { $! = EIO ; return } } my $fh = MockHandle->new; bzip2 $fh => \$dest; The point is that IO::Compress::Base fails to recognise that the read() has failed. Looking at the code where the comparison warnings occurred, there is code to handle read() failures but it doesn't work because it expects read() to return < 0 on failure, whereas in fact read() returns undef on failure. Correct that and I think the problem is solved, even for tied filehandles. Sorry for being unclear in the first place (and forgetting to set the bug report subject, ENEEDMORESLEEP :)

Sat Aug 01 04:03:03 2009 pmqs [...] cpan.org - Correspondence added

On Sat Aug 01 01:29:55 2009, PUBNOOP wrote: Show quoted text

> On Fri Jul 31 18:38:03 2009, PMQS wrote:

> > Hi Dave, > > > > thanks for the report. I've silenced the two numeric comparison > > warnings, but the "read() on closed filehandle" warning will be > > trickier/impractical to deal with.

> > IMO you don't want to silence that - if some fool feeds a closed > filehandle to IO::Compress::* as input then they should get warned about > it, just as if they'd read from a closed filehandle themselves.

Sorry - I wasn't very clear in my reply. When I said that the read on a closed filehandle would be trickier to deal with, I didn't mean that I wanted to silence it. Ideally that test (if it ever became practical to write) would be done upfront before getting anywhere near the read where a better warning/error could be output. Show quoted text

> I used a closed filehandle in the bug report as a quick way to cause a > failing read() call. I didn't mean that you should be silencing that > warning. To simulate a read() failure on an open filehandle you could: > > { > package MockHandle; > > use base qw/IO::String/; > use Errno qw/EIO/; > > sub READ { $! = EIO ; return } > } > > my $fh = MockHandle->new; > > bzip2 $fh => \$dest; > > The point is that IO::Compress::Base fails to recognise that the read() > has failed. Looking at the code where the comparison warnings occurred, > there is code to handle read() failures but it doesn't work because it > expects read() to return < 0 on failure, whereas in fact read() returns > undef on failure. Correct that and I think the problem is solved, even > for tied filehandles.

Yep - that's exactly the change I've made. I keep forgetting that the Perl read interface != C read interface. Show quoted text

> Sorry for being unclear in the first place (and forgetting to set the > bug report subject, ENEEDMORESLEEP :) >

Ditto - I shouldn't reply to mail late at night :-) Paul