From 0a41ca5a68626a0f44e0d552e460e86567e47140 Mon Sep 17 00:00:00 2001
From: Zefram <zefram@fysh.org>
Date: Wed, 15 Nov 2017 08:11:37 +0000
Subject: [PATCH] set $! when statting a closed filehandle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When a stat fails because it's on a closed or otherwise invalid
filehandle, $! was often not being set, depending on the operation
and the nature of the invalidity. Consistently set it to EBADF.
Fixes [perl #108288].
Petr Písař: Ported to 5.24.3.
Signed-off-by: Petr Písař <ppisar@redhat.com>
---
MANIFEST | 1 +
doio.c | 10 +++++++++-
pp_sys.c | 22 ++++++++++++---------
t/op/stat_errors.t | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 80 insertions(+), 10 deletions(-)
create mode 100644 t/op/stat_errors.t
diff --git a/MANIFEST b/MANIFEST
index fcf7eae..3077142 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -5394,6 +5394,7 @@ t/op/sselect.t See if 4 argument select works
t/op/stash.t See if %:: stashes work
t/op/state.t See if state variables work
t/op/stat.t See if stat works
+t/op/stat_errors.t See if stat and file tests handle threshold errors
t/op/study.t See if study works
t/op/studytied.t See if study works with tied scalars
t/op/sub_lval.t See if lvalue subroutines work
diff --git a/doio.c b/doio.c
index 2792c66..f2934c5 100644
--- a/doio.c
+++ b/doio.c
@@ -1429,8 +1429,11 @@ Perl_my_stat_flags(pTHX_ const U32 flags)
if (PL_op->op_flags & OPf_REF) {
gv = cGVOP_gv;
do_fstat:
- if (gv == PL_defgv)
+ if (gv == PL_defgv) {
+ if (PL_laststatval < 0)
+ SETERRNO(EBADF,RMS_IFI);
return PL_laststatval;
+ }
io = GvIO(gv);
do_fstat_have_io:
PL_laststype = OP_STAT;
@@ -1441,6 +1444,7 @@ Perl_my_stat_flags(pTHX_ const U32 flags)
int fd = PerlIO_fileno(IoIFP(io));
if (fd < 0) {
/* E.g. PerlIO::scalar has no real fd. */
+ SETERRNO(EBADF,RMS_IFI);
return (PL_laststatval = -1);
} else {
return (PL_laststatval = PerlLIO_fstat(fd, &PL_statcache));
@@ -1451,6 +1455,7 @@ Perl_my_stat_flags(pTHX_ const U32 flags)
}
PL_laststatval = -1;
report_evil_fh(gv);
+ SETERRNO(EBADF,RMS_IFI);
return -1;
}
else if ((PL_op->op_private & (OPpFT_STACKED|OPpFT_AFTER_t))
@@ -1503,6 +1508,8 @@ Perl_my_lstat_flags(pTHX_ const U32 flags)
if (cGVOP_gv == PL_defgv) {
if (PL_laststype != OP_LSTAT)
Perl_croak(aTHX_ "%s", no_prev_lstat);
+ if (PL_laststatval < 0)
+ SETERRNO(EBADF,RMS_IFI);
return PL_laststatval;
}
PL_laststatval = -1;
@@ -1512,6 +1519,7 @@ Perl_my_lstat_flags(pTHX_ const U32 flags)
"Use of -l on filehandle %"HEKf,
HEKfARG(GvENAME_HEK(cGVOP_gv)));
}
+ SETERRNO(EBADF,RMS_IFI);
return -1;
}
if ((PL_op->op_private & (OPpFT_STACKED|OPpFT_AFTER_t))
diff --git a/pp_sys.c b/pp_sys.c
index 5e0993d..2fcc219 100644
--- a/pp_sys.c
+++ b/pp_sys.c
@@ -2889,10 +2889,11 @@ PP(pp_stat)
Perl_croak(aTHX_ "The stat preceding lstat() wasn't an lstat");
}
- if (gv != PL_defgv) {
- bool havefp;
+ if (gv == PL_defgv) {
+ if (PL_laststatval < 0)
+ SETERRNO(EBADF,RMS_IFI);
+ } else {
do_fstat_have_io:
- havefp = FALSE;
PL_laststype = OP_STAT;
PL_statgv = gv ? gv : (GV *)io;
sv_setpvs(PL_statname, "");
@@ -2903,22 +2904,25 @@ PP(pp_stat)
if (IoIFP(io)) {
int fd = PerlIO_fileno(IoIFP(io));
if (fd < 0) {
+ report_evil_fh(gv);
PL_laststatval = -1;
SETERRNO(EBADF,RMS_IFI);
} else {
PL_laststatval = PerlLIO_fstat(fd, &PL_statcache);
- havefp = TRUE;
}
} else if (IoDIRP(io)) {
PL_laststatval =
PerlLIO_fstat(my_dirfd(IoDIRP(io)), &PL_statcache);
- havefp = TRUE;
} else {
+ report_evil_fh(gv);
PL_laststatval = -1;
+ SETERRNO(EBADF,RMS_IFI);
}
- }
- else PL_laststatval = -1;
- if (PL_laststatval < 0 && !havefp) report_evil_fh(gv);
+ } else {
+ report_evil_fh(gv);
+ PL_laststatval = -1;
+ SETERRNO(EBADF,RMS_IFI);
+ }
}
if (PL_laststatval < 0) {
@@ -3415,7 +3419,7 @@ PP(pp_fttty)
else if (name && isDIGIT(*name) && grok_atoUV(name, &uv, NULL) && uv <= PERL_INT_MAX)
fd = (int)uv;
else
- FT_RETURNUNDEF;
+ fd = -1;
if (fd < 0) {
SETERRNO(EBADF,RMS_IFI);
FT_RETURNUNDEF;
diff --git a/t/op/stat_errors.t b/t/op/stat_errors.t
new file mode 100644
index 0000000..e043c61
--- /dev/null
+++ b/t/op/stat_errors.t
@@ -0,0 +1,57 @@
+#!./perl
+
+BEGIN {
+ chdir 't' if -d 't';
+ require './test.pl';
+ set_up_inc('../lib');
+}
+
+plan(tests => 2*11*29);
+
+use Errno qw(EBADF ENOENT);
+
+open(SCALARFILE, "<", \"wibble") or die $!;
+open(CLOSEDFILE, "<", "./test.pl") or die $!;
+close(CLOSEDFILE) or die $!;
+opendir(CLOSEDDIR, "../lib") or die $!;
+closedir(CLOSEDDIR) or die $!;
+
+foreach my $op (
+ qw(stat lstat),
+ (map { "-$_" } qw(r w x o R W X O e z s f d l p S b c t u g k T B M A C)),
+) {
+ foreach my $arg (
+ (map { ($_, "\\*$_") }
+ qw(NEVEROPENED SCALARFILE CLOSEDFILE CLOSEDDIR _)),
+ "\"tmpnotexist\"",
+ ) {
+ my $argdesc = $arg;
+ if ($arg eq "_") {
+ my @z = lstat "tmpnotexist";
+ $argdesc .= " with prior stat fail";
+ }
+ SKIP: {
+ if ($op eq "-l" && $arg =~ /\A\\/) {
+ # The op weirdly stringifies the globref and uses it as
+ # a filename, rather than treating it as a file handle.
+ # That might be a bug, but while that behaviour exists it
+ # needs to be exempted from these tests.
+ skip "-l on globref", 2;
+ }
+ if ($op eq "-t" && $arg eq "\"tmpnotexist\"") {
+ # The op doesn't operate on filenames.
+ skip "-t on filename", 2;
+ }
+ $! = 0;
+ my $res = eval "$op $arg";
+ my $err = $!;
+ is $res, $op =~ /\A-/ ? undef : !!0, "result of $op $arg";
+ is 0+$err,
+ $arg eq "\"tmpnotexist\"" ||
+ ($op =~ /\A-[TB]\z/ && $arg =~ /_\z/) ? ENOENT : EBADF,
+ "error from $op $arg";
+ }
+ }
+}
+
+1;
--
2.13.6