Fatal.pm: context bug / patch

[Ilya Zakharevich]

[A complimentary Cc of this posting was sent to
Tom Regner
<regner@dievision.de>], 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?

[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.
>
> 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,

Indeed it does - thanks for the clarification.

> Ilya
>
> P.S. What do you think these parens about $call(@argv) do?

I thought it would enforce list-context to the call to original function -
your way of asking me indicates I'm mistaken :) -

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 make
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.

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 called in
list-context)[3] ?

kind regards,
Tom


[1]: [url]http://www.perlmonks.org/?node_id=542441[/url]

[2]: Why would I want to write:
opendir(my $dir, $start_dir);
eval {
while($_= readdir($dir)) {
print "$_ ";
$notempty = 1;
}
closedir $dir;
};
if ($@ and !$notempty) {
# handle empty dir
}

instead of:

opendir(my $dir, $start_dir);
while($_= readdir($dir)) {
print "$_ ";
$notempty = 1;
}
closedir $dir;
};
if (!$notempty) {
# handle empty dir
}

?

[3]:
use Fatal qw(readdir);
print "With Fatal:\n";
opendir($dir, $start_dir);
while(@compare = readdir($dir)) {
$count = 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 26

--
Dievision GmbH | Kriegerstrasse 44 | 30161 Hannover
Telefon: (0511) 288791-0 | Telefax: (0511) 288791-99
[url]http://www.dievision.de[/url]

[Ilya Zakharevich]

[A complimentary Cc of this posting was sent to
Tom Regner
<regner@dievision.de>], 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

[Ilya Zakharevich]

[A complimentary Cc of this posting was sent to
Tom Regner
<regner@dievision.de>], 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

[Tom Regner]

to provide context:

Ilya Zakharevich wrote (Message-ID:
<e2map5$29l$1@agate.berkeley.edu>):
[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:
<e2p3hv$10ri$1@agate.berkeley.edu>):
[A complimentary Cc of this posting was sent to
Tom Regner
<regner@dievision.de>], 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<Fatal> provides a way to conveniently replace functions which normally
return a false value when they fail
--

to

--
C<Fatal> 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
[url]http://www.dievision.de[/url]

[Ilya Zakharevich]

[A complimentary Cc of this posting was sent to
Tom Regner
<regner@dievision.de>], 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<Fatal> provides a way to conveniently replace functions which normally
> return a false value when they fail
> --
>
> to
>
> --
> C<Fatal> 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