Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.92k stars 549 forks source link

readdir() only returns one result when used with Fatal.pm #8381

Closed p5pRT closed 18 years ago

p5pRT commented 18 years ago

Migrated from rt.perl.org#38790 (status was 'resolved')

Searchable as RT38790$

p5pRT commented 18 years ago

From @tomhukins

Created by @tomhukins

Fatal.pm and readdir() do not play well together. Run the following script in a directory containing several files. It only prints out '.'. Then comment out the 'use Fatal' line and run the script again. It now prints out all the files in the directory.

#!/usr/bin/perl

use strict; use warnings;

use Fatal qw(readdir);

my $start_dir = '.'; opendir(my $dir\, $start_dir); my @​subdir = readdir $dir; closedir $dir; print "@​subdir\n";

Perl Info ``` Flags: category=library severity=medium Site configuration information for perl v5.8.7: Configured by tomh at Wed Aug 10 15:04:17 UTC 2005. Summary of my perl5 (revision 5 version 8 subversion 7) configuration: Platform: osname=freebsd, osvers=5.4-release, archname=i386-freebsd-64int uname='freebsd tomvmware.spirainternal.co.uk 5.4-release freebsd 5.4-release #0: sun may 8 10:21:06 utc 2005 root@harlow.cse.buffalo.edu:usrobjusrsrcsysgeneric i386 ' config_args='-sde -Dprefix=/usr/local -Darchlib=/usr/local/lib/perl5/5.8.7/mach -Dprivlib=/usr/local/lib/perl5/5.8.7 -Dman3dir=/usr/local/lib/perl5/5.8.7/perl/man/man3 -Dman1dir=/usr/local/man/man1 -Dsitearch=/usr/local/lib/perl5/site_perl/5.8.7/mach -Dsitelib=/usr/local/lib/perl5/site_perl/5.8.7 -Dscriptdir=/usr/local/bin -Dsiteman3dir=/usr/local/lib/perl5/5.8.7/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Ui_malloc -Ui_iconv -Uinstallusrbinperl -Dcc=cc -Duseshrplib -Dccflags=-DAPPLLIB_EXP="/usr/local/lib/perl5/5.8.7/BSDPAN" -Doptimize=-O -pipe -march=pentium4 -Ud_dosuid -Ui_gdbm -Dusethreads=n -Dusemymalloc=y -Duse64bitint' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=define use64bitall=undef uselongdouble=undef usemymalloc=y, bincompat5005=undef Compiler: cc='cc', ccflags ='-DAPPLLIB_EXP="/usr/local/lib/perl5/5.8.7/BSDPAN" -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -I/usr/local/include', optimize='-O -pipe -march=pentium4', cppflags='-DAPPLLIB_EXP="/usr/local/lib/perl5/5.8.7/BSDPAN" -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -I/usr/local/include' ccversion='', gccversion='3.4.2 [FreeBSD] 20040728', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags ='-pthread -Wl,-E -L/usr/local/lib' libpth=/usr/lib /usr/local/lib libs=-lm -lcrypt -lutil perllibs=-lm -lcrypt -lutil libc=, so=so, useshrplib=true, libperl=libperl.so gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags=' -Wl,-R/usr/local/lib/perl5/5.8.7/mach/CORE' cccdlflags='-DPIC -fPIC', lddlflags='-shared -L/usr/local/lib' Locally applied patches: defined-or @INC for perl v5.8.7: /usr/local/lib/perl5/site_perl/5.8.7 /usr/local/lib/perl5/site_perl/5.8.7/mach /usr/local/lib/perl5/site_perl /usr/local/lib/perl5/5.8.7/BSDPAN /usr/local/lib/perl5/5.8.7/mach /usr/local/lib/perl5/5.8.7 . Environment for perl v5.8.7: HOME=/home/tomh LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/games:/usr/local/sbin:/usr/local/bin:/usr/X11R6/bin:/home/tomh/bin PERL_BADLANG (unset) SHELL=/usr/local/bin/zsh ```
p5pRT commented 18 years ago

From @rgs

Tom Hukins (via RT) wrote​:

Fatal.pm and readdir() do not play well together. Run the following script in a directory containing several files. It only prints out '.'. Then comment out the 'use Fatal' line and run the script again. It now prints out all the files in the directory.

#!/usr/bin/perl

use strict; use warnings;

use Fatal qw(readdir);

my $start_dir = '.'; opendir(my $dir\, $start_dir); my @​subdir = readdir $dir; closedir $dir; print "@​subdir\n";

Fatal will replace readdir() by this function :

  sub (*) {   local($"\, $!) = ('\, '\, 0);   CORE​::readdir($_[0]) || croak "Can't readdir(@​_)​: $!";   }

As you see\, this construct forces readdir to be called in scalar context; which is why it returns only one element\, as documented in perlfunc.

We could add a wantarray() before that\, but that would alter the proper behaviour of other weird code like this :

  use Fatal qw(open);   my @​useless_array = open my $fh\, "non-existent-file";

Other ideas ?

-- No matter what scheme is chosen\, most of the world is unhappy.   -- Jeff Johnson in rpm-devel

p5pRT commented 18 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 18 years ago

From @ysth

On Thu\, Mar 23\, 2006 at 02​:03​:57PM +0100\, Rafael Garcia-Suarez wrote​:

Tom Hukins (via RT) wrote​:

Fatal.pm and readdir() do not play well together. Run the following script in a directory containing several files. It only prints out '.'. Then comment out the 'use Fatal' line and run the script again. It now prints out all the files in the directory.

#!/usr/bin/perl

use strict; use warnings;

use Fatal qw(readdir);

my $start_dir = '.'; opendir(my $dir\, $start_dir); my @​subdir = readdir $dir; closedir $dir; print "@​subdir\n";

Fatal will replace readdir() by this function :

sub \(\*\) \{
    local\($"\, $\!\) = \('\, '\, 0\);
    CORE​::readdir\($\_\[0\]\) || croak "Can't readdir\(@​\_\)​: $\!";
\}

As you see\, this construct forces readdir to be called in scalar context; which is why it returns only one element\, as documented in perlfunc.

We could add a wantarray() before that\, but that would alter the proper behaviour of other weird code like this :

use Fatal qw\(open\);
my @​useless\_array = open my $fh\, "non\-existent\-file";

Other ideas ?

*Is* there any way to detect a failed list-context readdir()? If not\, Fatal should continue to do what it is doing and the limitation should be documented.

p5pRT commented 18 years ago

From @rgs

Yitzchak Scott-Thoennes wrote​:

*Is* there any way to detect a failed list-context readdir()?

Due to the iterative nature of readdir()\, I doubt Fatal makes sense on it\, from a language point of view. But strictly speaking\, readdir() fails when there are no more entries to read\, that is\, by returning undef in scalar context\, and an empty list in list context.

If not\, Fatal should continue to do what it is doing and the limitation should be documented.

That would be probably the best solution\, but I wonder whether other builtins could cause the same kind of problems. A glance through the keyword list doesn't give other ideas. (Maybe select()\, which returns -1 on error.) There is also a minor inconsistency on some error values : some return 0 on error\, some return undef (because 0 could be returned in case of success -- e.g. read()). So\, short of harcoding all exceptions\, there is no good fix.

p5pRT commented 18 years ago

From @jbenjore

On 3/23/06\, Rafael Garcia-Suarez \rgarciasuarez@​mandriva\.com wrote​:

Yitzchak Scott-Thoennes wrote​:

*Is* there any way to detect a failed list-context readdir()?

Due to the iterative nature of readdir()\, I doubt Fatal makes sense on it\, from a language point of view. But strictly speaking\, readdir() fails when there are no more entries to read\, that is\, by returning undef in scalar context\, and an empty list in list context.

If not\, Fatal should continue to do what it is doing and the limitation should be documented.

That would be probably the best solution\, but I wonder whether other builtins could cause the same kind of problems. A glance through the keyword list doesn't give other ideas. (Maybe select()\, which returns -1 on error.) There is also a minor inconsistency on some error values : some return 0 on error\, some return undef (because 0 could be returned in case of success -- e.g. read()). So\, short of harcoding all exceptions\, there is no good fix.

So hardcode the exceptions. They're well established and limited. Heck\, for readdir()\, if it returned an empty list/undef\, why isn't $! being examined to decide failure? It isn't that failure is undetectable\, it just isn't a case of looking at the return value because any of empty/false/undef/-1/0 are possible as non-error returns.

It could be considered the responsibility of a core module like Fatal to handle all the various exceptional return values. I sure do. I also see no reason why Fatal can't do the right thing. So why is it that Fatal has to just document this as a limitation? It's possible for Fatal to handle list context readdir\, right? If so\, it should.

Josh

p5pRT commented 18 years ago

From @rgs

Joshua ben Jore wrote​:

So hardcode the exceptions. They're well established and limited. Heck\, for readdir()\, if it returned an empty list/undef\, why isn't $! being examined to decide failure? It isn't that failure is undetectable\, it just isn't a case of looking at the return value because any of empty/false/undef/-1/0 are possible as non-error returns.

Not sure\, I think all directories have always at least one entries. At least on Unixes.

It could be considered the responsibility of a core module like Fatal to handle all the various exceptional return values. I sure do. I also see no reason why Fatal can't do the right thing. So why is it that Fatal has to just document this as a limitation? It's possible for Fatal to handle list context readdir\, right? If so\, it should.

Yes\, seems reasonable. and patches speak louder than words :)

p5pRT commented 18 years ago

From @jbenjore

On 3/23/06\, Rafael Garcia-Suarez \rgarciasuarez@​mandriva\.com wrote​:

Joshua ben Jore wrote​:

Fatal has to just document this as a limitation? It's possible for Fatal to handle list context readdir\, right? If so\, it should.

Yes\, seems reasonable. and patches speak louder than words :)

I was just considering which evening I wanted to give up for this. I want to get the modules that are in perl's core lint-safe. I suppose I'd prefer this patch first. :-/ I do have some tweaks to lint outstanding and will send them on when I've got the tests written.

Josh

p5pRT commented 18 years ago

From @wb8tyw

Rafael Garcia-Suarez wrote​:

Joshua ben Jore wrote​:

So hardcode the exceptions. They're well established and limited. Heck\, for readdir()\, if it returned an empty list/undef\, why isn't $! being examined to decide failure? It isn't that failure is undetectable\, it just isn't a case of looking at the return value because any of empty/false/undef/-1/0 are possible as non-error returns.

Not sure\, I think all directories have always at least one entries. At least on Unixes.

OpenVMS directories can be empty and frequently are.

-John wb8tyw@​qsl.net Personal Opinion Only

p5pRT commented 18 years ago

From @rgs

Change #28040 documents this as a bug in Fatal.pm.

p5pRT commented 18 years ago

@rgs - Status changed from 'open' to 'resolved'

p5pRT commented 17 years ago

From lallip@cs.rpi.edu

Created by lallip@cs.rpi.edu

When using the standard Fatal module\, readdir called in list context acts like readdir called in scalar context. That is\, it only returns the "next" directory entry\, rather than all remaining directory entries. An example​:

$ ls temp/ foo.1 foo.2 foo.3 $ perl -wle' use Fatal qw/readdir/; opendir my $dh\, "temp" or die $!; my @​files = readdir($dh); print for @​files; ' . $ perl -wle' #use Fatal qw/readdir/; opendir my $dh\, "temp" or die $!; my @​files = readdir($dh); print for @​files; ' . .. foo.1 foo.2 foo.3

To verify the normal scalar semantics are in play\, I modified the first execution like so​: $ perl -wle' use Fatal qw/readdir/; opendir my $dh\, "temp" or die $!; my @​files = readdir($dh); push @​files\, readdir($dh); push @​files\, readdir($dh); print for @​files; ' . .. foo.1

When Fatal creates the new versions of these core functions\, calling context should be preserved.

Perl Info ``` Flags: category=library severity=medium Site configuration information for perl v5.8.4: Configured by cchaves at Mon Jun 21 17:02:10 EDT 2004. Summary of my perl5 (revision 5 version 8 subversion 4) configuration: Platform: osname=solaris, osvers=2.9, archname=sun4-solaris uname='sunos pitddb1.alb.flis1.com 5.9 generic_112233-12 sun4u sparc sunw,sun-fire-480r ' config_args='' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='/opt/SUNWspro/bin/cc', ccflags ='-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O', cppflags='-I/usr/local/include' ccversion='Forte Developer 7 C 5.4 2002/03/09', gccversion='', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=4321 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='/opt/SUNWspro/bin/cc', ldflags =' -L/usr/lib -L/usr/ccs/lib -L/opt/SUNWspro/prod/lib -L/usr/local/lib ' libpth=/usr/lib /usr/ccs/lib /opt/SUNWspro/prod/lib /usr/local/lib libs=-lsocket -lnsl -ldl -lm -lc perllibs=-lsocket -lnsl -ldl -lm -lc libc=/lib/libc.so, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags=' ' cccdlflags='-KPIC', lddlflags='-G -L/usr/lib -L/usr/ccs/lib -L/opt/SUNWspro/prod/lib -L/usr/local/lib' Locally applied patches: @INC for perl v5.8.4: /home/plalli/pitd/nyspitaccums/lib /home/plalli/pitd/comintsysperl/lib /home/plalli/lib /home/plalli/lib/perl5/site_perl/5.8.4/sun4-solaris /home/plalli/lib/perl5/site_perl/5.8.4/sun4-solaris /home/plalli/lib/perl5/site_perl/5.8.4 /home/plalli/lib/perl5/site_perl/5.8.4/sun4-solaris/auto/ /home/plalli/lib/perl5/5.8.4//sun4-solaris /home/plalli/lib/perl5/5.8.4/ /opt2/Perl5_8_4/lib/perl5/5.8.4/sun4-solaris /opt2/Perl5_8_4/lib/perl5/5.8.4 /opt2/Perl5_8_4/lib/perl5/site_perl/5.8.4/sun4-solaris /opt2/Perl5_8_4/lib/perl5/site_perl/5.8.4 /opt2/Perl5_8_4/lib/perl5/site_perl . Environment for perl v5.8.4: HOME=/home/plalli LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH=::IFXBEGIN:/opt2/informix/lib/sparc:/opt2/informix/lib/d mi:/opt2/informix/lib/esql:/opt2/informix/lib:/opt2/ifam/slib:IFXEND LOGDIR=/home/plalli/log PATH=/opt2/perl/bin:/opt/SUNWspro/bin:.:/usr/bin:/opt:/opt/WorkShop/SUNW spro/bin:/opt/SUNWspro/bin:/usr/sbin:/usr/bin:/usr/ccs/bin:/usr/sfw/bin: /usr/local/bin:/usr/ucb:/usr/openwin/bin:/usr/dt/bin:/opt/EMCpower/bin:/ opt/Navisphere/bin:/opt/OV/bin:/opt/SUNWexplo/bin:/opt/SUNWsan/bin:/opt/ SUNWspro/bin:/opt/VRTS/bin:/opt/VRTSalloc/bin:/opt/VRTSat/bin:/opt/VRTSa t57/bin:/opt/VRTSaz/bin:/opt/VRTSaz57/bin:/opt/VRTScpi/bin:/opt/VRTSfspr o/bin:/opt/VRTSob/bin:/opt/VRTSperl/bin:/opt/VRTSvcs/bin:/opt/VRTSvlic/b in:/opt/VRTSvmpro/bin:/opt/dcelocal/bin:/opt/perf/bin:/opt/perl/bin:/opt /perl5.8/bin:/opt/schily/bin:/opt/sfw/bin:/opt/ssh2/bin:/opt/xalan-j_2_4 _0/bin:/etc:/opt/EMCpower/bin/sparcv9:/etc/emc/bin:/usr/openwin/bin:/usr /ucb:/etc:/usr/ccs/bin:/home/rlinster/setup:/usr/dt/bin:/bin:/usr/local/ bin:.:/usr/bin:/opt:/opt/WorkShop/SUNWspro/bin:/opt/SUNWspro/bin:/usr/sb in:/usr/bin:/usr/ccs/bin:/usr/sfw/bin:/usr/local/bin:/usr/ucb:/usr/openw in/bin:/usr/dt/bin:/opt/EMCpower/bin:/opt/Navisphere/bin:/opt/OV/bin:/op t/SUNWexplo/bin:/opt/SUNWsan/bin:/opt/SUNWspro/bin:/opt/VRTS/bin:/opt/VR TSalloc/bin:/opt/VRTSat/bin:/opt/VRTSat57/bin:/opt/VRTSaz/bin:/opt/VRTSa z57/bin:/opt/VRTScpi/bin:/opt/VRTSfspro/bin:/opt/VRTSob/bin:/opt/VRTSper l/bin:/opt/VRTSvcs/bin:/opt/VRTSvlic/bin:/opt/VRTSvmpro/bin:/opt/dceloca l/bin:/opt/perf/bin:/opt/perl/bin:/opt/perl5.8/bin:/opt/schily/bin:/opt/ sfw/bin:/opt/ssh2/bin:/opt/xalan-j_2_4_0/bin:/etc:/opt/EMCpower/bin/spar cv9:/etc/emc/bin:/usr/openwin/bin:/usr/ucb:/etc:/usr/ccs/bin:/home/rlins ter/setup:/usr/dt/bin:/bin:/usr/local/bin:/usr/local/parasoft/bin.solari s::/scratch/d01/perl5/bin:/home/plalli/bin:.::/home/informix/local_bin:/ home/plalli/bin:/home/plalli/bin/bin:/home/plalli/setup:/patches/solaris /2005-1/9/111719-03/SPROws/reloc/SUNWspro/WS6U2/bin:.:IFXBEGIN:/usr/bin: /opt/SUNWspro/bin:/usr/ccs/bin:/opt2/informix/bin:IFXEND:/home/informix/ local_bin PERL5LIB=/home/plalli/pitd/nyspitaccums/lib:/home/plalli/pitd/comintsysp erl/lib:/home/plalli/lib:/home/plalli/lib/perl5/site_perl/5.8.4/sun4-sol aris:/home/plalli/lib/perl5/site_perl/5.8.4:/home/plalli/lib/perl5/site_ perl/5.8.4/sun4-solaris/auto/:/home/plalli/lib/perl5/5.8.4/ PERL_BADLANG (unset) SHELL=/bin/ksh ```
p5pRT commented 17 years ago

From @tamias

On Thu\, Aug 09\, 2007 at 07​:02​:43AM -0700\, lallip @​ cs. rpi. edu wrote​:

When using the standard Fatal module\, readdir called in list context acts like readdir called in scalar context. That is\, it only returns the "next" directory entry\, rather than all remaining directory entries.

Here's the debug output from Fatal​:

# _make_fatal​: sub=main​::readdir pkg=main name=readdir void=0

sub (*) {   local($"\, $!) = ('\, '\, 0);   CORE​::readdir($_[0]) || croak "Can't readdir(@​_)​: $!"; }

Obviously\, that code is not going to work for readdir().

perldoc Fatal​:

  "Fatal" provides a way to conveniently replace functions which   normally return a false value when they fail with equivalents which   raise exceptions if they are not successful. This lets you use   these functions without having to test their return values   explicitly on each call.

readdir() can return a false value on success (either a file named '0'\, or an undef or empty list when there are no more entries). Thus\, I'd say the answer is that you should not be using Fatal with readdir(). Fatal is meant to be used with functions like open() and close()\, which behave the same in scalar or list context and return false only on failure.

Ronald

p5pRT commented 17 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 17 years ago

From @rgs

On 09/08/07\, via RT lallip @​ cs. rpi. edu \perlbug\-followup@​perl\.org wrote​:

When using the standard Fatal module\, readdir called in list context acts like readdir called in scalar context. That is\, it only returns the "next" directory entry\, rather than all remaining directory entries. An example​:

That has been documented as a known bug\, not fixable; see bug #38790 for details.

p5pRT commented 17 years ago

From jns@gellyfish.com

On Thu\, 2007-08-09 at 18​:48 +0200\, Rafael Garcia-Suarez wrote​:

On 09/08/07\, via RT lallip @​ cs. rpi. edu \perlbug\-followup@​perl\.org wrote​:

When using the standard Fatal module\, readdir called in list context acts like readdir called in scalar context. That is\, it only returns the "next" directory entry\, rather than all remaining directory entries. An example​:

That has been documented as a known bug\, not fixable; see bug #38790 for details.

Would it be possible to change the documentation for Fatal to make it clear that this is an unfixable mis-feature in the module rather than in perl in general?

/J\

p5pRT commented 17 years ago

From @rgs

On 09/08/07\, Jonathan Stowe \jns@​gellyfish\.com wrote​:

Would it be possible to change the documentation for Fatal to make it clear that this is an unfixable mis-feature in the module rather than in perl in general?

Certainly\, if someone provides a better wording for that "BUGS" section I added.