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