develooper Front page | perl.perl5.porters | Postings from April 2013

Re: [perl #117595] Perl 5.16 regression: lost warning for -l on filehandle

Thread Previous
From:
Brian Fraser
Date:
April 29, 2013 03:38
Subject:
Re: [perl #117595] Perl 5.16 regression: lost warning for -l on filehandle
Message ID:
CA+nL+nYoBYJ9buGtkpgnS0ynC9pXmNW+jJj-vDU0nHwuXjJzUQ@mail.gmail.com
On Sun, Apr 28, 2013 at 10:07 PM, Father Chrysostomos via RT <
perlbug-followup@perl.org> wrote:

> On Mon Apr 15 18:56:31 2013, xdg@xdg.me wrote:
> > Here is a patch for blead Perl with tests, but part of it is a bit
> > crude.  -l on filehandles should warn *unless* there is string
> > overloading on the handle.  I wrote the patch with SvGAMAGIC, but it
> > really should be more specific to string overloading and I don't know
> > how to do that in XS/C.
> >
> > So someone more wizardly than I should tweak this before it gets applied.
> >
> > David
> >
> >
> >
> >
> > On Sun, Apr 14, 2013 at 2:07 AM, David Golden <xdg@xdg.me> wrote:
> > > On Sat, Apr 13, 2013 at 4:14 PM, Brad Gilbert via RT
> > > <perlbug-comment@perl.org> wrote:
> > >> Based on the commit message it may be related to
> > >>
> http://perl5.git.perl.org/perl.git/commit/433644eed8ac93495dfaad947c1503
> > >> ce219b414b
> > >
> > > Based on the commit message, that's pretty clearly the wrong fix:
> > >
> > > Historical behavior of C<-l $handle>:
> > >
> > >     5.6: treat as filename
> > >
> > >     5.8 to 5.14:
> > >         - without warnings: treat as filename
> > >         - with warnings: return undef and warn
> > >
> > >     5.16: treat as filename
> > >
> > > The desired behavior would seem to be:
> > >     - without warnings: return undef
> > >     - with warnings: return undef and warn
>
> To me, it makes sense to keep the filename treatment.  But if you really
> want to warn, I have no problem with that.
>
> I have two reasons for wanting to keep the filename treatment:
> 1) Code that does ‘no warnings; -l $foo’ will continue to behave exactly
> the same way as it has since 5.6.
> 2) We avoid the problem of having to detect string overloading, which
> itself turns into a new source of unexpected behaviour.
>
> Now, concerning your patch:
>
> > From 4a300c01ecacf421b35116b56ed930f8a2a512ec Mon Sep 17 00:00:00 2001
> > From: David Golden <dagolden@cpan.org>
> > Date: Mon, 15 Apr 2013 11:44:04 +0100
> > Subject: [PATCH] Restore warning for -l on filehandles
> >
> > Filehandles are no longer treated as names for -l. Instead, calling -l
> > on a filehandle returns undef to signal that it's an invalid operation.
> > If warnings are on, a warning is issued as well.
> >
> > Other filetests let globs stringify via overloading, so this patch does
> > not prevent calling -l on an overloaded handle, though my implementation
> > for that is probably not the best.
> > ---
> >  doio.c              |  8 ++++++++
> >  t/lib/warnings/doio |  7 ++++++-
> >  t/op/filetest.t     | 17 ++++++++++++-----
> >  3 files changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/doio.c b/doio.c
> > index 4e8d48a..bca8838 100644
> > --- a/doio.c
> > +++ b/doio.c
> > @@ -1359,6 +1359,14 @@ Perl_my_lstat_flags(pTHX_ const U32 flags)
> >
> >      PL_laststype = OP_LSTAT;
> >      PL_statgv = NULL;
> > +    /* XXX this should check for stringification overloading, not just
> > +     * any sort of magic */
>
> Furthermore, the use of the first G in SvGAMAGIC is unnecessary, as
> get-magic will already have been called here (from memory; I didn’t
> check).  Also, it is wrong, because -l $foo and -l $tied should behave
> the same way.
>
> I don’t remember offhand how to check for specific overload types.  But
> searching for _amg_ (or _amt_?) in the source tree should find some
> examples.
>
> > +    if (SvROK(TOPs) && SvTYPE(SvRV(TOPs)) == SVt_PVIO && !
> SvGAMAGIC(TOPs)) {
>
> Here you are checking that you have an ioref (*foo{IO}), so it doesn’t
> apply to $fh as in open my $fh..., which is a globref.  It also doesn’t
> apply to -l *foo.  What you need is something more like
> if(isGV_with_GP(TOPs) || (SvROK(TOPs) && (SvTYPE(SvRV(TOPs)) == SVt_PVIO
> || isGV_with_GP(SvRV(TOPs))))).
>
> (Maybe we should make a macro out of that.)
>
> > +     if ( ckWARN(WARN_IO) )
> > +            Perl_warner(aTHX_ packWARN(WARN_IO), "Use of -l on
> filehandle %s",
> > +             GvENAME((const GV *)SvRV(TOPs)));
>
> And GvENAME only applies to globs, so we would have to handle IO
> thingies specially here (omit the " %s" part of the message), to avoid
> this:
>
> $ ./perl -Ilib -we '-l *STDOUT{IO}'
> Segmentation fault: 11
>
> (with your patch).
>

As a minor addendum, that's not UTF8-clean either.

Thread Previous


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About