Front page | perl.perl5.porters |
Postings from July 2014
Re: [perl #119555] require should not clobber $! and $^E on success(impact on autouse and autodie)
Thread Previous
|
Thread Next
From:
Christian Millour
Date:
July 28, 2014 23:11
Subject:
Re: [perl #119555] require should not clobber $! and $^E on success(impact on autouse and autodie)
Message ID:
53D6D8A6.7000105@abtela.com
minor edit on the patch file to reference the proper RT ticket (119...
instead of 199...). Sorry for the noise.
Christian
Le 29/07/2014 00:46, Christian Millour a écrit :
> Le 28/07/2014 05:01, Craig A. Berry a écrit :
>> The problem is my patch doesn't actually do what you want. It does
>> successfully restore any errno that is set in pp_require while rifling
>> through @INC trying to find the file being required (assuming the file
>> is eventually found and successfully opened).
>>
>> But after it's opened, the file is sent to S_doeval to be compiled,
>> and somewhere in there errno gets cleared again. I haven't figured
>> out exactly how or where yet. There is a call to CLEAR_ERRSV(), which
>> has been there in some form since Perl 5.000. That might be what's
>> clearing errno, or not; I don't know. It doesn't look like it would,
>> but maybe there's magic on PL_errgv or something.
>>
>> On the surface, it sounds reasonable that a successful require should
>> leave errno in the state it was in prior to the require. But nothing
>> resembling a complete or well-though-out implementation has been
>> proposed, and any change in this area will involve touching sensitive
>> bits of the core that have been as they are for twenty years. A
>> reformulated patch is attached, but at best it's a partial solution.
>
> Thank you for your time and efforts on this.
>
> pp_require is indeed quite involved and I fully understand anyone being
> wary of touching it.
>
> Still, according to the documentation, require dies on error, and
> otherwise survives. So irrespective of the internal causes of changes to
> $! and $^E, it should be OK to restore their values on entry in those
> cases where require does not die. In Perl :
>
> sub new_require {
> my ($e, $se) = ($!, $^E);
> old_require $_[0]; # dies on error...
> ($!, $^E) = ($e, $se); # ... and otherwise restore $! and $^E
> }
>
> While not strictly equivalent, and as an experiment, I wrapped the
> existing pp_require, renamed pp_require1 but otherwise untouched, into a
> new pp_require tasked with saving/restoring $! and $^E :
>
> PP(pp_require1)
> {
> ... existing code
> }
> PP(pp_require)
> {
> OP *op;
> dSAVE_ERRNO;
> op = Perl_pp_require1(
> #ifndef PERL_IS_MINIPERL
> my_perl
> #endif
> );
> if ((*_errno()) == 0)
> RESTORE_ERRNO;
> return op;
> }
>
> However crude and ill-mannered, this hack seemed to work. A variation on
> the above might be the best solution. A similar idea, implemented in the
> attached patch, is to dSAVE_ERRNO as you did, but to RESTORE_ERRNO
> before any return point that is not a DIE. I find four of those : two
> RETPUSHYES, one RETPUSHUNDEF, and the final return. I can't see how
> those RESTORE_ERRNOs could damage the working of pp_require, even with
> the recursive calls that seem to occur (via S_doeval). Note that the
> SETERRNO(0, SS_NORMAL) introduced in
> <http://perl5.git.perl.org/perl.git/commit/d8bfb8bddf933a815b590823bd52295534e6ded0?f=pp_ctl.c>
> has been removed because 1) it seems unrelated to the internal mechanism
> of pp_require, and 2) its effect would be nullified by the
> RESTORE_ERRNOs (at least on success).
>
> This seems to work (one.pm is successfully required, without clobbering
> $! and $^E) :
>
> blead(w32) $ echo 0 > zero.pm; echo 1 > one.pm
> blead(w32) $ ../perl -IPerl -IRule -e 'for (@ARGV) { $! = $^E = 1; eval
> {require "$_"; 1} or print $@; print $_,q{ ==> $! = }, 0+$!, q{, $^E =
> }, 0+$^E, qq{ [$^E]\n}}' doesnotexist.pm zero.pm one.pm
> Can't locate doesnotexist.pm in @INC (you may need to install the
> doesnotexist module) (@INC contains: Perl Rule
> D:/perls/blead/perl-git2/lib .) at -e line 1.
> doesnotexist.pm ==> $! = 2, $^E = 2 [Le fichier sp cifi est introuvable]
> zero.pm did not return a true value at -e line 1.
> zero.pm ==> $! = 1, $^E = 1 [Fonction incorrecte]
> one.pm ==> $! = 1, $^E = 1 [Fonction incorrecte]
> blead(w32) $
>
> This is better than the stock perl (here strawberry perl portable 5.20)
>
> Perl_5.20.0 $ echo 0 > zero.pm; echo 1 > one.pm
> Perl_5.20.0 $ perl -IPerl -IRule -e 'for (@ARGV) { $! = $^E = 1; eval
> {require "$_"; 1} or print $@; print $_,q{ ==> $! = }, 0+$!, q{, $^E =
> }, 0+$^E, qq{ [$^E]\n}}' doesnotexist.pm zero.pm one.pm
> Can't locate doesnotexist.pm in @INC (you may need to install the
> doesnotexist module) (@INC contains: Perl Rule
> C:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/site/lib/MSWin32-x86-multi-thread-64int
> C:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/site/lib
> C:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/vendor/lib
> C:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/lib .) at -e line 1.
> doesnotexist.pm ==> $! = 2, $^E = 2 [Le fichier spécifié est introuvable]
> zero.pm did not return a true value at -e line 1.
> zero.pm ==> $! = 0, $^E = 6 [Descripteur non valide]
> one.pm ==> $! = 0, $^E = 6 [Descripteur non valide]
> Perl_5.20.0 $
>
> In case of failure the values of $! and $^E do not seem much worse with
> the patch than without (given that for "did not return a true value"
> they are not set anyway, so may be a bit random...).
>
> Comments/critics/smokes warmly welcome :-) Best regards,
>
> Christian
>
>
>
>
>
>
Thread Previous
|
Thread Next