Bug #6247 for Archive-Tar: Archive::Tar->new($file) doesn't return undef on failure

Thu May 06 17:10:19 2004 Guest - Ticket created

Subject:

Archive::Tar->new($file) doesn't return undef on failure

I'm using something like: $tar = Archive::Tar->new($file); die unless defined $tar; print "$tar\n"; on a file with permissions `--w-------'. I get this output: Could not create filehandle for 'install.tar': Permission denied! at ./install.pl line 111 Archive::Tar=HASH(0x101025d0) The warning is good, but it should die on the next line. The print command shows that $tar isn't undef, but a hash reference. This is a critical bug for my script, because I add files and write the tar file back to disk. The orginal contents of the tar file are lost. It looks to me that simply checking what the read method returns and passing undef rather than $obj would do the trick. As an aside, I would welcome a feature simular to DBI's RaiseError, which causes failures to die rather than warn. Thanks, Jon Ericson This is using Archive-Tar-1.08.tar.gz from CPAN. System information follows: $ perl -v This is perl, v5.8.4 built for cygwin Copyright 1987-2004, Larry Wall Perl may be copied only under the terms of either the Artistic License or the GNU General Public License, which may be found in the Perl 5 source kit. Complete documentation for Perl, including FAQ lists, should be found on this system using `man perl' or `perldoc perl'. If you have access to the Internet, point your browser at http://www.perl.com/, the Perl Home Page. $ uname -a CYGWIN_NT-5.0 Jon-Ericson 1.5.9(0.112/4/2) 2004-03-18 23:05 i686 unknown unknown Cygwin

Fri May 07 02:59:37 2004 kane [...] dwim.org - Correspondence added

From:	"Jos I. Boumans" <kane [...] dwim.org>
Subject:	Re: [cpan #6247] Archive::Tar->new($file) doesn't return undef on failure
Date:	Fri, 7 May 2004 08:59:58 +0200
To:	bug-Archive-Tar [...] rt.cpan.org
RT-Send-Cc:

On May 6, 2004, at 11:10 PM, Guest via RT wrote: Show quoted text

> I'm using something like: > > $tar = Archive::Tar->new($file); > die unless defined $tar; > print "$tar\n"; > > on a file with permissions `--w-------'. I get this output: > > Could not create filehandle for 'install.tar': Permission denied! at > ./install.pl line 111 > Archive::Tar=HASH(0x101025d0) > > The warning is good, but it should die on the next line. The print > command shows that $tar isn't undef, but a hash reference.

That is most interesting, and something i can not reproduce on my system. The code that generates the warning is at line 192 of Archive::Tar and reads: unless( $fh->open( $file, $mode ) ) { $self->_error( qq[Could not create filehandle for '$file': $!!] ); return; } as you can see, it simply returns and therefor should be false. Perhaps you can send me a test case that demonstrates this problem -- a simple, short script and a source tar file that raises the exception you are looking for. Show quoted text

> As an aside, I would welcome a feature simular to DBI's RaiseError, > which causes failures to die rather than warn.

That's an interesting idea. I will see if i can add it to the next version. -- Jos Boumans "You know you are never more indignant in life than when you're shopping at a store you feel is beneath you and one of the other customers mistakes you for one of the employees of that store." - Dennis Miller CPANPLUS http://cpanplus.sf.net

Wed May 12 17:23:30 2004 JERICSON [...] cpan.fsck.com - Correspondence added

From:

Jon Ericson

[kane@dwim.org - Fri May 7 02:59:37 2004]: Show quoted text

> That is most interesting, and something i can not reproduce on my > system. > > The code that generates the warning is at line 192 of Archive::Tar and > reads: > > unless( $fh->open( $file, $mode ) ) { > $self->_error( qq[Could not create filehandle for '$file': $!!] > ); > return; > } > > as you can see, it simply returns and therefor should be false.

Correct. But this return is not checked by the new() method: sub new { ### copying $tmpl here since a shallow copy makes it use the ### same aref, causing for files to remain in memory always. my $obj = bless { _data => [ ], _file => 'Unknown' }, shift; $obj->read( @_ ) if @_; return $obj; } I think line 93 should read something like: if (@_) { unless ($obj->read( @_ )){ return; }; }; Show quoted text

> Perhaps you can send me a test case that demonstrates this problem -- a > simple, short script and a source tar file that raises the exception > you are looking for.

How about: $ touch temp.tar $ chmod -r temp.tar $ ls -l temp.tar --w------- 1 jericson mkgroup_ 0 May 7 10:30 temp.tar $ perl -MArchive::Tar -e '$tar = Archive::Tar->new("temp.tar"); die unless defined $tar;print "$tar\n";die if $tar->error' Could not create filehandle for 'temp.tar': Permission denied! at -e line 1 Archive::Tar=HASH(0x100f12e0) Died at -e line 1. With my change above, I get: $ perl -MArchive::Tar -e '$tar = Archive::Tar->new("temp.tar"); die unless defined $tar;print "$tar\n";die if $tar->error' Could not create filehandle for 'temp.tar': Permission denied! at -e line 1 Died at -e line 1. P.S. Sorry if this shows up multiple times. I haven't seen any updates to this bug when I use email. Jon

Sat May 22 06:44:04 2004 kane [...] dwim.org - Correspondence added

From:	"Jos I. Boumans" <kane [...] dwim.org>
Subject:	Re: [cpan #6247] Archive::Tar->new($file) doesn't return undef on failure
Date:	Sat, 22 May 2004 12:44:17 +0200
To:	bug-Archive-Tar [...] rt.cpan.org
RT-Send-Cc:

On May 12, 2004, at 11:23 PM, Jon Ericson via RT wrote: Show quoted text

>> as you can see, it simply returns and therefor should be false.

> > Correct. But this return is not checked by the new() method:

Thanks, and fixed @10625. This will be in the 1.09 release, which will be be uploaded to CPAN shortly. -- Jos Boumans How do I prove I am not crazy to people who are? CPANPLUS http://cpanplus.sf.net

Mon May 24 17:27:29 2004 JERICSON [...] cpan.fsck.com - Correspondence added

From:

jericson [...] cpan.org

[kane@dwim.org - Sat May 22 06:44:04 2004]: Show quoted text

> > On May 12, 2004, at 11:23 PM, Jon Ericson via RT wrote:

> >> as you can see, it simply returns and therefor should be false.

> > > > Correct. But this return is not checked by the new() method:

> > Thanks, and fixed @10625. > This will be in the 1.09 release, which will be be uploaded to CPAN > shortly.

1.09 looks good to me. Thanks, Jon

Tue May 25 14:38:04 2004 JERICSON [...] cpan.fsck.com - Correspondence added

From:

jericson [...] cpan.org

[kane@dwim.org - Sat May 22 06:44:04 2004]: Show quoted text

> > On May 12, 2004, at 11:23 PM, Jon Ericson via RT wrote:

> >> as you can see, it simply returns and therefor should be false.

> > > > Correct. But this return is not checked by the new() method:

> > Thanks, and fixed @10625. > This will be in the 1.09 release, which will be be uploaded to CPAN > shortly.

1.09 looks good to me. Thanks, Jon

Tue May 25 15:01:25 2004 kane [...] cpan.org - Status changed from 'new' to 'resolved'