New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Perl 5.16 regression: lost warning for -l on filehandle #12911
Comments
From @xdgI found a regression bug in Perl. Up to Perl 5.16, use of "-l" on a file (or directory) handle would Use of -l on filehandle %s That warning is no longer issued in Perl 5.16 Test file attached. David -- |
From @b2gillsOn Sat Apr 13 01:38:38 2013, xdg@xdg.me wrote:
Presumably this is because of some bug fixes related to -l Based on the commit message it may be related to At any rate here is a example program which shows that it is now using use strict; open my $fh, '<', $0; On 5.14 it says Use of -l on filehandle $fh at test.pl line 7. on 5.16 it says "GLOB(0x25f4938)" is a symlink |
From [Unknown Contact. See original ticket]On Sat Apr 13 01:38:38 2013, xdg@xdg.me wrote:
Presumably this is because of some bug fixes related to -l Based on the commit message it may be related to At any rate here is a example program which shows that it is now using use strict; open my $fh, '<', $0; On 5.14 it says Use of -l on filehandle $fh at test.pl line 7. on 5.16 it says "GLOB(0x25f4938)" is a symlink |
From @xdgOn Sat, Apr 13, 2013 at 4:14 PM, Brad Gilbert via RT
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: 5.16: treat as filename The desired behavior would seem to be: |
From @xdgHere is a patch for blead Perl with tests, but part of it is a bit 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:
-- |
From @xdg0001-Restore-warning-for-l-on-filehandles.patchFrom 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 */
+ if (SvROK(TOPs) && SvTYPE(SvRV(TOPs)) == SVt_PVIO && ! SvGAMAGIC(TOPs)) {
+ if ( ckWARN(WARN_IO) )
+ Perl_warner(aTHX_ packWARN(WARN_IO), "Use of -l on filehandle %s",
+ GvENAME((const GV *)SvRV(TOPs)));
+ return (PL_laststatval = -1);
+ }
file = SvPV_flags_const_nolen(TOPs, flags);
sv_setpv(PL_statname,file);
PL_laststatval = PerlLIO_lstat(file,&PL_statcache);
diff --git a/t/lib/warnings/doio b/t/lib/warnings/doio
index 732f66d..f4a211a 100644
--- a/t/lib/warnings/doio
+++ b/t/lib/warnings/doio
@@ -157,12 +157,17 @@ Unsuccessful stat on filename containing newline at - line 3.
Unsuccessful stat on filename containing newline at - line 4.
########
# doio.c [Perl_my_stat]
+open $fh, $0 or die "# $!";
use warnings 'io';
-l STDIN;
+-l $fh;
no warnings 'io';
-l STDIN;
+-l $fh;
+close $fh;
EXPECT
-Use of -l on filehandle STDIN at - line 3.
+Use of -l on filehandle STDIN at - line 4.
+Use of -l on filehandle $fh at - line 5.
########
# doio.c [Perl_my_stat]
use utf8;
diff --git a/t/op/filetest.t b/t/op/filetest.t
index 9ab049f..f7166a1 100644
--- a/t/op/filetest.t
+++ b/t/op/filetest.t
@@ -9,7 +9,7 @@ BEGIN {
require './test.pl';
}
-plan(tests => 49 + 27*14);
+plan(tests => 51 + 27*14);
# Tests presume we are in t/op directory and that file 'TEST' is found
# therein.
@@ -109,10 +109,17 @@ SKIP: {
# Since we already have our skip block set up, we might as well put this
# test here, too:
# -l always treats a non-bareword argument as a file name
- system 'ln', '-s', $ro_empty_file, \*foo;
- local $^W = 1;
- is(-l \*foo, 1, '-l \*foo is a file name');
- unlink \*foo;
+ my $linkfile = tempfile();
+ system 'ln', '-s', $ro_empty_file, $linkfile;
+ open my $fh, '<', $linkfile or die "open $linkfile: $!";
+ is(-l $fh, undef, '-l HANDLE gives undef');
+ unlink $linkfile;
+
+ system 'ln', '-s', $ro_empty_file, "\\*foo";
+ system 'ls -l';
+ is(-l \*foo, undef, '-l \*foo gives undef');
+ is(-l "\\*foo", 1, '-l "\*foo" works');
+ unlink "\\*foo";
}
# test that _ is a bareword after filetest operators
--
1.8.2
|
@jkeenan - Status changed from 'new' to 'open' |
From @cpansproutOn Mon Apr 15 18:56:31 2013, xdg@xdg.me wrote:
http://perl5.git.perl.org/perl.git/commit/433644eed8ac93495dfaad947c1503
To me, it makes sense to keep the filename treatment. But if you really I have two reasons for wanting to keep the filename treatment: Now, concerning your patch:
Furthermore, the use of the first G in SvGAMAGIC is unnecessary, as I don’t remember offhand how to check for specific overload types. But
Here you are checking that you have an ioref (*foo{IO}), so it doesn’t (Maybe we should make a macro out of that.)
And GvENAME only applies to globs, so we would have to handle IO $ ./perl -Ilib -we '-l *STDOUT{IO}' (with your patch).
As I noted above, I would prefer that we simply omit that statement, for I’m surprised I even got enough time to review your patch. I’m afraid I (BTW, in case they are reading this, thank you to everyone who picked up
I understand this patch is unfinished, but make sure the final version
That last bit seems pointless. Surely you want to be testing "".\*foo, no?
-- Father Chrysostomos |
From @HugmeirOn Sun, Apr 28, 2013 at 10:07 PM, Father Chrysostomos via RT <
As a minor addendum, that's not UTF8-clean either. |
From @rjbsFC's reasons for treating things as a string seem reasonable to me. I have just pushed 934fd9a It would probably be a good idea for someone to review this patch! It involves me looking at All tests successful, including tests restored and added. -- |
From @cpansproutOn Tue May 07 16:05:48 2013, rjbs wrote:
Two things: 1) The error is not utf8-clean. %s" should be %"HEKf and GvENAME should 2) The warning is only produced for globrefs, not globs or iorefs. Now, -- Father Chrysostomos |
From @rjbsMarked non-blocking, and will look at further improvement after the thaw. -- |
From @cpansproutOn Thu May 09 09:37:28 2013, sprout wrote:
I’ve addressed these remaining issues in commit 5840701. -- Father Chrysostomos |
@cpansprout - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#117595 (status was 'resolved')
Searchable as RT117595$
The text was updated successfully, but these errors were encountered: