Fatal.pm: context bug / patch
Fatal.pm: context bug / patch
am 25.04.2006 11:32:31 von Tom Regner
Hello,
(I sent this message, by reflex, to the mentioned Author
(Lionel.Cons@cern.ch)=20
first, and as a second thaught to bug-Fatal@rt.cpan.org, relazing too l=
ate
that Fatal as a core-Module is not a distribution covered via rt.cpan -=
-
appologies if it reaches somebody twice)
following a discussion on perlmonks[1], I tried to fix the problem that=
=20
readdir isn't "fatalizable" without losing correct context-awareness:=20=
Fatalized readdir returns only a single element even if in list-context=
, as=20
is illustrated with this small programm:
[0923]tom@mo perl $ cat readfatal.pl
#!/usr/bin/perl
use strict;
use warnings;
$| =3D 1;
my $start_dir =3D '.';
my ($dir, @compare, $count);
print "Without Fatal:\n";
opendir($dir, $start_dir);
@compare =3D readdir($dir);
$count =3D 0;
for (@compare) {
=A0 =A0 =A0 =A0 print "" . $count++ . ": " . $_ . "\n";
}
closedir $dir;
print "\n";
use Fatal qw(readdir);
print "With Fatal:\n";
opendir($dir, $start_dir);
@compare =3D readdir($dir);
$count =3D 0;
for (@compare) {
=A0 =A0 =A0 =A0 print "" . $count++ . ": " . $_ . "\n";
}
closedir $dir;
Producing:
0926]tom@mo perl $ perl readfatal.pl
Without Fatal:
0: .
1: ..
2: pm
3: Fatal.pm
4: readfatal.pl
5: kbs.png
6: xmas.pl
[...]
52: noaccess
53: big-homepage.log
54: grepcount.pl
55: kbs-we.png
With Fatal:
0: .
This small patch solves this problem by forcing listcontext if wantarra=
y is=20
true and the context isn't void (assuming $void is set if the call is m=
ade
in=20
void context, otherwise the first branch of the if($void) statement in =
sub=20
one_invocation must be adapted accordingly):
*** /usr/lib/perl5/5.8.8/Fatal.pm =A0 =A0 =A0 2006-04-25 09:20:38.00000=
0000 +0200
--- Fatal.pm =A0 =A02006-04-25 09:31:53.000000000 +0200
***************
*** 80,86 ****
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 $call(@argv) || croak "Can't $name(\@_)=
/ .
=A0 =A0 =A0 =A0 =A0 =A0 =A0($core ? ': $!' : ', \$! is \"$!\"') . '"'
=A0 =A0 } else {
! =A0 =A0 return qq{$call(@argv) || croak "Can't $name(\@_)} .
=A0 =A0 =A0 =A0 =A0 =A0 =A0($core ? ': $!' : ', \$! is \"$!\"') . '"';
=A0 =A0 }
=A0 }
--- 80,86 ----
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 $call(@argv) || croak "Can't $name(\@_)=
/ .
=A0 =A0 =A0 =A0 =A0 =A0 =A0($core ? ': $!' : ', \$! is \"$!\"') . '"'
=A0 =A0 } else {
! =A0 =A0 return qq{wantarray ? ($call(@argv)) : $call(@argv) || croak =
"Can't=20
$name(\@_)} .
=A0 =A0 =A0 =A0 =A0 =A0 =A0($core ? ': $!' : ', \$! is \"$!\"') . '"';
=A0 =A0 }
=A0 }
using a patched Fatal.pm:
[0933]tom@mo perl $ perl -I. readfatal.pl
Without Fatal:
0: .
1: ..
2: pm
3: Fatal.pm
4: readfatal.pl
[...]
53: big-homepage.log
54: grepcount.pl
55: kbs-we.png
With Fatal:
0: .
1: ..
2: pm
3: Fatal.pm
[...]
52: noaccess
53: big-homepage.log
54: grepcount.pl
55: kbs-we.png
I don't know the procedure, if you would be so kind as to inform me if =
you=20
consider this patch or why it's complete and utter bs or even that you
simply=20
don't have the time to care about it I'd be grateful
kind regards,
Tom Regner
[1]: http://www.perlmonks.org/?node_id=3D542441
--=20
Dievision GmbH | Kriegerstrasse 44 | 30161 Hannover
Telefon: (0511) 288791-0 | Telefax: (0511) 288791-99
http://www.dievision.de
Re: Fatal.pm: context bug / patch
am 26.04.2006 01:19:01 von Ilya Zakharevich
[A complimentary Cc of this posting was sent to
Tom Regner
], who wrote in article <444decaf$0$5508$4d3ebbfe@news1.pop-hannover.net>:
> *** /usr/lib/perl5/5.8.8/Fatal.pm =A0 =A0 =A0 2006-04-25 09:20:38.00000=
> 0000 +0200
> --- Fatal.pm =A0 =A02006-04-25 09:31:53.000000000 +0200
> ***************
> *** 80,86 ****
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 $call(@argv) || croak "Can't $name(\@_)=
> / .
> =A0 =A0 =A0 =A0 =A0 =A0 =A0($core ? ': $!' : ', \$! is \"$!\"') . '"'
> =A0 =A0 } else {
> ! =A0 =A0 return qq{$call(@argv) || croak "Can't $name(\@_)} .
> =A0 =A0 =A0 =A0 =A0 =A0 =A0($core ? ': $!' : ', \$! is \"$!\"') . '"';
> =A0 =A0 }
> =A0 }
> --- 80,86 ----
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 $call(@argv) || croak "Can't $name(\@_)=
> / .
> =A0 =A0 =A0 =A0 =A0 =A0 =A0($core ? ': $!' : ', \$! is \"$!\"') . '"'
> =A0 =A0 } else {
> ! =A0 =A0 return qq{wantarray ? ($call(@argv)) : $call(@argv) || croak =
> "Can't=20
> $name(\@_)} .
> =A0 =A0 =A0 =A0 =A0 =A0 =A0($core ? ': $!' : ', \$! is \"$!\"') . '"';
> =A0 =A0 }
> =A0 }
This patch is a no-no-no. Fatal should be used only for functions
which return scalar value, and indicate failure by returning false.
Basically, you just disabled Fatal for function calls in list context.
A function which returns a scalar should behave the same in list and
scalar context; your patch breaks this.
Hope this helps,
Ilya
P.S. What do you think these parens about $call(@argv) do?
Re: Fatal.pm: context bug / patch
am 26.04.2006 09:27:13 von Tom Regner
Ilya Zakharevich wrote:
> [A complimentary Cc of this posting was sent to
> Tom Regner
[...]
> This patch is a no-no-no. Fatal should be used only for functions
> which return scalar value, and indicate failure by returning false.
>=20
> Basically, you just disabled Fatal for function calls in list context=
..
> A function which returns a scalar should behave the same in list and
> scalar context; your patch breaks this.
>=20
> Hope this helps,
Indeed it does - thanks for the clarification.
> Ilya
>=20
> P.S. What do you think these parens about $call(@argv) do?
I thought it would enforce list-context to the call to original functio=
n -
your way of asking me indicates I'm mistaken :) -=20
To clarify: I didn't really tried to understand the module, I just set =
out
to provide a solution for the original post made on perlmonks[1] (same =
link
as mentioned in my original post) - even if a Fatal readdir() didn't ma=
ke
much sense to me[2]. OTOH the Fatal POD did not suggest to me that
readdir()s behaviour would break - ("...replace functions which normall=
y
return a false value when they fail with equivalents which raise
exceptions..." - readdir does this both in scalar- and list-context.
I think I really don't get "it", but despite of that: is it possible to=
reword/supplement Fatal.pm POD with hinds as to the behaviour in cases =
like
readdir() (that it behaves like called in scalar-context, even if calle=
d in
list-context)[3] ?
kind regards,
Tom
[1]: http://www.perlmonks.org/?node_id=3D542441
[2]: Why would I want to write:
opendir(my $dir, $start_dir);
eval {
while($_=3D readdir($dir)) {
print "$_ ";
$notempty =3D 1;
}
closedir $dir;
};
if ($@ and !$notempty) {
# handle empty dir
}
instead of:
opendir(my $dir, $start_dir);
while($_=3D readdir($dir)) {
print "$_ ";
$notempty =3D 1;
}
closedir $dir;
};
if (!$notempty) {
# handle empty dir
}
?
[3]:
use Fatal qw(readdir);
print "With Fatal:\n";
opendir($dir, $start_dir);
while(@compare =3D readdir($dir)) {
$count =3D 0;
for (@compare) {
print "" . $count++ . ": " . $_ . "\n";
}
}
closedir $dir;
With Fatal:
0: .
0: ..
0: pm
0: readfatal.pl
0: kbs.png
[...]
0: grepcount.pl
0: kbs-we.png
Can't readdir(GLOB(0x814280)): Ungültiger Dateideskriptor at (eval 1)=
line 3
main::__ANON__('GLOB(0x8142180)') called at readfatal.pl line 2=
6
--=20
Dievision GmbH | Kriegerstrasse 44 | 30161 Hannover
Telefon: (0511) 288791-0 | Telefax: (0511) 288791-99
http://www.dievision.de
Re: Fatal.pm: context bug / patch
am 27.04.2006 02:34:07 von Ilya Zakharevich
[A complimentary Cc of this posting was sent to
Tom Regner
], who wrote in article <444f20d1$0$26380$4d3ebbfe@news1.pop-hannover.net>:
> much sense to me[2]. OTOH the Fatal POD did not suggest to me that
> readdir()s behaviour would break - ("...replace functions which normally
> return a false value when they fail with equivalents which raise
> exceptions..." - readdir does this both in scalar- and list-context.
Your description of behaviour of readdir() in list context is wrong.
> I think I really don't get "it", but despite of that: is it possible
> to= reword/supplement Fatal.pm POD with hinds as to the behaviour in
> cases = like readdir() (that it behaves like called in
> scalar-context, even if calle= d in list-context)[3] ?
Sure. Any clarification to the docs is welcome - provided it is
written AS IF the writers understand what they write about. ;-)
Thanks,
Ilya
Re: readdir() API broken?
am 27.04.2006 02:39:54 von Ilya Zakharevich
[A complimentary Cc of this posting was sent to
Tom Regner
], who wrote in article <444f20d1$0$26380$4d3ebbfe@news1.pop-hannover.net>:
> To clarify: I didn't really tried to understand the module, I just
> set = out to provide a solution for the original post made on
> perlmonks[1] (same = link as mentioned in my original post) - even
> if a Fatal readdir() didn't ma= ke much sense to me[2].
What you noticed is a fundamental flow of Perl's readdir()
list-context API: there is no simple way to distinguish failing call
from an empty (rest of a) directory.
As an absolute minimum, one needs a snippet in the docs which
disambiguates between these two cases (probably using $! ?).
Thanks,
Ilya
Re: Fatal.pm: context bug / patch
am 27.04.2006 11:50:30 von Tom Regner
to provide context:
Ilya Zakharevich wrote (Message-ID:
):
[concerning my blatantly ignorant patch]
> This patch is a no-no-no. Fatal should be used only for functions
> which return scalar value, and indicate failure by returning false.
Ilya Zakharevich wrote (Message-ID:
):
[A complimentary Cc of this posting was sent to
Tom Regner
], who wrote in article
<444f20d1$0$26380$4d3ebbfe@news1.pop-hannover.net>:
[...]
>> I think I really don't get "it", but despite of that: is it possible
>> to= reword/supplement Fatal.pm POD with hinds as to the behaviour in
>> cases = like readdir() (that it behaves like called in
>> scalar-context, even if calle= d in list-context)[3] ?
>
> Sure. Any clarification to the docs is welcome - provided it is
> written AS IF the writers understand what they write about. ;-)
1): I did dig deeper now into Fatal.pm and think I have at least a grasp of
whats going on :-), but couldn't come up with a way to behave like expected
with "fatalizing" readdir()..., which, as you mentioned (see 2), shouldn't
be done anyway...
2): Your first remark to my original post should be in the POD:
> Fatal should be used only for functions
> which return scalar value, and indicate failure by returning false.
I suggest to change
--
C provides a way to conveniently replace functions which normally
return a false value when they fail
--
to
--
C provides a way to conveniently replace functions which
return a scalar value and normally indicate failure with returning a false
value
--
I'm not a native english speaker, but I hope this is understandable and not
to bad; It would have spared the original poster on perlmonks and me a bit
of confusion on why the code behaves the way it does.
Kind regards and thanks for your time and effort,
Tom Regner
--
Dievision GmbH | Kriegerstrasse 44 | 30161 Hannover
Telefon: (0511) 288791-0 | Telefax: (0511) 288791-99
http://www.dievision.de
Re: Fatal.pm: context bug / patch
am 28.04.2006 05:05:26 von Ilya Zakharevich
[A complimentary Cc of this posting was sent to
Tom Regner
], who wrote in article <445093e6$0$27399$4d3ebbfe@news1.pop-hannover.net>:
> 2): Your first remark to my original post should be in the POD:
>
> > Fatal should be used only for functions
> > which return scalar value, and indicate failure by returning false.
>
> I suggest to change
>
> --
> C provides a way to conveniently replace functions which normally
> return a false value when they fail
> --
>
> to
>
> --
> C provides a way to conveniently replace functions which
> return a scalar value and normally indicate failure with returning a false
> value
> --
>
> I'm not a native english speaker, but I hope this is understandable and not
> to bad; It would have spared the original poster on perlmonks and me a bit
> of confusion on why the code behaves the way it does.
Looks reasonable.
Please send results of `diff -pu' to perl5-porters mailing list.
Thanks,
Ilya