a4ac56
From 8c7182b26a43f14cd8afbfbe4448cbbd691c3609 Mon Sep 17 00:00:00 2001
a4ac56
From: Zefram <zefram@fysh.org>
a4ac56
Date: Wed, 15 Nov 2017 08:11:37 +0000
a4ac56
Subject: [PATCH] set $! when statting a closed filehandle
a4ac56
MIME-Version: 1.0
a4ac56
Content-Type: text/plain; charset=UTF-8
a4ac56
Content-Transfer-Encoding: 8bit
a4ac56
a4ac56
When a stat fails because it's on a closed or otherwise invalid
a4ac56
filehandle, $! was often not being set, depending on the operation
a4ac56
and the nature of the invalidity.  Consistently set it to EBADF.
a4ac56
Fixes [perl #108288].
a4ac56
a4ac56
Petr Písař: Ported to 5.26.1.
a4ac56
a4ac56
Signed-off-by: Petr Písař <ppisar@redhat.com>
a4ac56
---
a4ac56
 MANIFEST           |  1 +
a4ac56
 doio.c             | 10 +++++++++-
a4ac56
 pp_sys.c           | 22 ++++++++++++---------
a4ac56
 t/op/stat_errors.t | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
a4ac56
 4 files changed, 80 insertions(+), 10 deletions(-)
a4ac56
 create mode 100644 t/op/stat_errors.t
a4ac56
a4ac56
diff --git a/MANIFEST b/MANIFEST
a4ac56
index fcbf5cc..996759e 100644
a4ac56
--- a/MANIFEST
a4ac56
+++ b/MANIFEST
a4ac56
@@ -5670,6 +5670,7 @@ t/op/srand.t			See if srand works
a4ac56
 t/op/sselect.t			See if 4 argument select works
a4ac56
 t/op/stash.t			See if %:: stashes work
a4ac56
 t/op/stat.t			See if stat works
a4ac56
+t/op/stat_errors.t		See if stat and file tests handle threshold errors
a4ac56
 t/op/state.t			See if state variables work
a4ac56
 t/op/study.t			See if study works
a4ac56
 t/op/studytied.t		See if study works with tied scalars
a4ac56
diff --git a/doio.c b/doio.c
a4ac56
index 70d7747..71dc6e4 100644
a4ac56
--- a/doio.c
a4ac56
+++ b/doio.c
a4ac56
@@ -1437,8 +1437,11 @@ Perl_my_stat_flags(pTHX_ const U32 flags)
a4ac56
     if (PL_op->op_flags & OPf_REF) {
a4ac56
 	gv = cGVOP_gv;
a4ac56
       do_fstat:
a4ac56
-        if (gv == PL_defgv)
a4ac56
+        if (gv == PL_defgv) {
a4ac56
+	    if (PL_laststatval < 0)
a4ac56
+		SETERRNO(EBADF,RMS_IFI);
a4ac56
             return PL_laststatval;
a4ac56
+	}
a4ac56
 	io = GvIO(gv);
a4ac56
         do_fstat_have_io:
a4ac56
         PL_laststype = OP_STAT;
a4ac56
@@ -1449,6 +1452,7 @@ Perl_my_stat_flags(pTHX_ const U32 flags)
a4ac56
                 int fd = PerlIO_fileno(IoIFP(io));
a4ac56
                 if (fd < 0) {
a4ac56
                     /* E.g. PerlIO::scalar has no real fd. */
a4ac56
+		    SETERRNO(EBADF,RMS_IFI);
a4ac56
                     return (PL_laststatval = -1);
a4ac56
                 } else {
a4ac56
                     return (PL_laststatval = PerlLIO_fstat(fd, &PL_statcache));
a4ac56
@@ -1459,6 +1463,7 @@ Perl_my_stat_flags(pTHX_ const U32 flags)
a4ac56
         }
a4ac56
 	PL_laststatval = -1;
a4ac56
 	report_evil_fh(gv);
a4ac56
+	SETERRNO(EBADF,RMS_IFI);
a4ac56
 	return -1;
a4ac56
     }
a4ac56
     else if ((PL_op->op_private & (OPpFT_STACKED|OPpFT_AFTER_t))
a4ac56
@@ -1511,6 +1516,8 @@ Perl_my_lstat_flags(pTHX_ const U32 flags)
a4ac56
 	if (cGVOP_gv == PL_defgv) {
a4ac56
 	    if (PL_laststype != OP_LSTAT)
a4ac56
 		Perl_croak(aTHX_ "%s", no_prev_lstat);
a4ac56
+	    if (PL_laststatval < 0)
a4ac56
+		SETERRNO(EBADF,RMS_IFI);
a4ac56
 	    return PL_laststatval;
a4ac56
 	}
a4ac56
 	PL_laststatval = -1;
a4ac56
@@ -1520,6 +1527,7 @@ Perl_my_lstat_flags(pTHX_ const U32 flags)
a4ac56
 		              "Use of -l on filehandle %" HEKf,
a4ac56
 			      HEKfARG(GvENAME_HEK(cGVOP_gv)));
a4ac56
 	}
a4ac56
+	SETERRNO(EBADF,RMS_IFI);
a4ac56
 	return -1;
a4ac56
     }
a4ac56
     if ((PL_op->op_private & (OPpFT_STACKED|OPpFT_AFTER_t))
a4ac56
diff --git a/pp_sys.c b/pp_sys.c
a4ac56
index fefbea3..87961f1 100644
a4ac56
--- a/pp_sys.c
a4ac56
+++ b/pp_sys.c
a4ac56
@@ -2925,10 +2925,11 @@ PP(pp_stat)
a4ac56
 		Perl_croak(aTHX_ "The stat preceding lstat() wasn't an lstat");
a4ac56
 	}
a4ac56
 
a4ac56
-	if (gv != PL_defgv) {
a4ac56
-	    bool havefp;
a4ac56
+	if (gv == PL_defgv) {
a4ac56
+	    if (PL_laststatval < 0)
a4ac56
+		SETERRNO(EBADF,RMS_IFI);
a4ac56
+	} else {
a4ac56
           do_fstat_have_io:
a4ac56
-	    havefp = FALSE;
a4ac56
 	    PL_laststype = OP_STAT;
a4ac56
 	    PL_statgv = gv ? gv : (GV *)io;
a4ac56
             SvPVCLEAR(PL_statname);
a4ac56
@@ -2939,22 +2940,25 @@ PP(pp_stat)
a4ac56
                     if (IoIFP(io)) {
a4ac56
                         int fd = PerlIO_fileno(IoIFP(io));
a4ac56
                         if (fd < 0) {
a4ac56
+			    report_evil_fh(gv);
a4ac56
                             PL_laststatval = -1;
a4ac56
                             SETERRNO(EBADF,RMS_IFI);
a4ac56
                         } else {
a4ac56
                             PL_laststatval = PerlLIO_fstat(fd, &PL_statcache);
a4ac56
-                            havefp = TRUE;
a4ac56
                         }
a4ac56
                     } else if (IoDIRP(io)) {
a4ac56
                         PL_laststatval =
a4ac56
                             PerlLIO_fstat(my_dirfd(IoDIRP(io)), &PL_statcache);
a4ac56
-                        havefp = TRUE;
a4ac56
                     } else {
a4ac56
+			report_evil_fh(gv);
a4ac56
                         PL_laststatval = -1;
a4ac56
+			SETERRNO(EBADF,RMS_IFI);
a4ac56
                     }
a4ac56
-            }
a4ac56
-	    else PL_laststatval = -1;
a4ac56
-	    if (PL_laststatval < 0 && !havefp) report_evil_fh(gv);
a4ac56
+            } else {
a4ac56
+		report_evil_fh(gv);
a4ac56
+		PL_laststatval = -1;
a4ac56
+		SETERRNO(EBADF,RMS_IFI);
a4ac56
+	    }
a4ac56
         }
a4ac56
 
a4ac56
 	if (PL_laststatval < 0) {
a4ac56
@@ -3451,7 +3455,7 @@ PP(pp_fttty)
a4ac56
     else if (name && isDIGIT(*name) && grok_atoUV(name, &uv, NULL) && uv <= PERL_INT_MAX)
a4ac56
         fd = (int)uv;
a4ac56
     else
a4ac56
-	FT_RETURNUNDEF;
a4ac56
+	fd = -1;
a4ac56
     if (fd < 0) {
a4ac56
         SETERRNO(EBADF,RMS_IFI);
a4ac56
 	FT_RETURNUNDEF;
a4ac56
diff --git a/t/op/stat_errors.t b/t/op/stat_errors.t
a4ac56
new file mode 100644
a4ac56
index 0000000..e043c61
a4ac56
--- /dev/null
a4ac56
+++ b/t/op/stat_errors.t
a4ac56
@@ -0,0 +1,57 @@
a4ac56
+#!./perl
a4ac56
+
a4ac56
+BEGIN {
a4ac56
+    chdir 't' if -d 't';
a4ac56
+    require './test.pl';
a4ac56
+    set_up_inc('../lib');
a4ac56
+}
a4ac56
+
a4ac56
+plan(tests => 2*11*29);
a4ac56
+
a4ac56
+use Errno qw(EBADF ENOENT);
a4ac56
+
a4ac56
+open(SCALARFILE, "<", \"wibble") or die $!;
a4ac56
+open(CLOSEDFILE, "<", "./test.pl") or die $!;
a4ac56
+close(CLOSEDFILE) or die $!;
a4ac56
+opendir(CLOSEDDIR, "../lib") or die $!;
a4ac56
+closedir(CLOSEDDIR) or die $!;
a4ac56
+
a4ac56
+foreach my $op (
a4ac56
+    qw(stat lstat),
a4ac56
+    (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)),
a4ac56
+) {
a4ac56
+    foreach my $arg (
a4ac56
+	(map { ($_, "\\*$_") }
a4ac56
+	    qw(NEVEROPENED SCALARFILE CLOSEDFILE CLOSEDDIR _)),
a4ac56
+	"\"tmpnotexist\"",
a4ac56
+    ) {
a4ac56
+	my $argdesc = $arg;
a4ac56
+	if ($arg eq "_") {
a4ac56
+	    my @z = lstat "tmpnotexist";
a4ac56
+	    $argdesc .= " with prior stat fail";
a4ac56
+	}
a4ac56
+	SKIP: {
a4ac56
+	    if ($op eq "-l" && $arg =~ /\A\\/) {
a4ac56
+		# The op weirdly stringifies the globref and uses it as
a4ac56
+		# a filename, rather than treating it as a file handle.
a4ac56
+		# That might be a bug, but while that behaviour exists it
a4ac56
+		# needs to be exempted from these tests.
a4ac56
+		skip "-l on globref", 2;
a4ac56
+	    }
a4ac56
+	    if ($op eq "-t" && $arg eq "\"tmpnotexist\"") {
a4ac56
+		# The op doesn't operate on filenames.
a4ac56
+		skip "-t on filename", 2;
a4ac56
+	    }
a4ac56
+	    $! = 0;
a4ac56
+	    my $res = eval "$op $arg";
a4ac56
+	    my $err = $!;
a4ac56
+	    is $res, $op =~ /\A-/ ? undef : !!0, "result of $op $arg";
a4ac56
+	    is 0+$err,
a4ac56
+		$arg eq "\"tmpnotexist\"" ||
a4ac56
+		    ($op =~ /\A-[TB]\z/ && $arg =~ /_\z/) ? ENOENT : EBADF,
a4ac56
+		"error from $op $arg";
a4ac56
+	}
a4ac56
+    }
a4ac56
+}
a4ac56
+
a4ac56
+1;
a4ac56
-- 
a4ac56
2.13.6
a4ac56