On 11/25/2016 06:00 PM, Paul Eggert wrote:
Isn't that a bug in glibc freopen? It shouldn't fail ... merely
because stdin doesn't have a valid file descriptor. If so, we should
fix the Gnulib freopen module to work around the bug.
I did that, by installing the attached patches into Gnulib (first patch)
and into Coreutils (2nd and 3rd patches). This fixes the shuf bug for
me, so closing the bug report. CC'ing to bug-gnulib due to the Gnulib fix.
diff --git a/ChangeLog b/ChangeLog
index 26eb3d5..a7e03f2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2016-11-26 Paul Eggert <egg...@cs.ucla.edu>
+
+ freopen: work around glibc bug with closed fd
+ Work around glibc bug#15589, where freopen mishandles the case
+ where stdin etc. are already closed.
+ * doc/posix-functions/freopen.texi (freopen): Document the bug.
+ * lib/freopen.c (_GL_ALREADY_INCLUDING_STDIO_H): Define this
+ instead of __need_FILE, as the latter does not work with glibc.
+ Include <fcntl.h>, for open flags.
+ (rpl_freopen): Work around glibc bug.
+ * m4/freopen.m4 (gl_FUNC_FREOPEN): Check for bug.
+ * modules/freopen (Depends-on): Add fcntl-h.
+ * tests/test-freopen.c (main): Test for bug.
+
2016-11-25 Paul Eggert <egg...@cs.ucla.edu>
fnmatch: fix typo introduced on 2016-08-17
diff --git a/doc/posix-functions/freopen.texi b/doc/posix-functions/freopen.texi
index d14c3e1..f79a05a 100644
--- a/doc/posix-functions/freopen.texi
+++ b/doc/posix-functions/freopen.texi
@@ -9,6 +9,10 @@ Gnulib module: freopen
Portability problems fixed by Gnulib:
@itemize
@item
+On some platforms, if @code{stream} does not already have an open
+file descriptor, @code{freopen} returns the stream without opening
+the file: glibc 2.24.
+@item
On platforms where @code{off_t} is a 32-bit type, @code{freopen} may not work
correctly with files larger than 2 GB. (Cf. @code{AC_SYS_LARGEFILE}.)
@item
diff --git a/lib/freopen.c b/lib/freopen.c
index 4cf7528..229c1d9 100644
--- a/lib/freopen.c
+++ b/lib/freopen.c
@@ -19,12 +19,12 @@
/* If the user's config.h happens to include <stdio.h>, let it include only
the system's <stdio.h> here, so that orig_freopen doesn't recurse to
rpl_freopen. */
-#define __need_FILE
+#define _GL_ALREADY_INCLUDING_STDIO_H
#include <config.h>
/* Get the original definition of freopen. It might be defined as a macro. */
#include <stdio.h>
-#undef __need_FILE
+#undef _GL_ALREADY_INCLUDING_STDIO_H
#include <errno.h>
@@ -39,29 +39,54 @@ orig_freopen (const char *filename, const char *mode, FILE *stream)
this include because of the preliminary #include <stdio.h> above. */
#include "stdio.h"
+#include <fcntl.h>
#include <string.h>
+#include <unistd.h>
FILE *
rpl_freopen (const char *filename, const char *mode, FILE *stream)
{
FILE *result;
-
#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
- if (filename != NULL && strcmp (filename, "/dev/null") == 0)
- filename = "NUL";
+ char const *null_device = "NUL";
+ if (filename && strcmp (filename, "/dev/null") == 0)
+ filename = null_device;
+#else
+ char const *null_device = "/dev/null";
#endif
- /* Clear errno to check the success of freopen() with it */
+#ifdef __KLIBC__
errno = 0;
+#endif
result = orig_freopen (filename, mode, stream);
+ if (!result)
+ {
#ifdef __KLIBC__
- /* On OS/2 kLIBC, freopen() returns NULL even if it is successful
- if filename is NULL. */
- if (!filename && !result && !errno)
- result = stream;
+ /* On OS/2 kLIBC, freopen returns NULL even if it is successful
+ if filename is NULL. */
+ if (!filename && !errno)
+ result = stream;
#endif
+ }
+ else if (filename)
+ {
+ int fd = fileno (result);
+ if (dup2 (fd, fd) < 0 && errno == EBADF)
+ {
+ int nullfd = open (null_device, O_RDONLY | O_CLOEXEC);
+ int err = 0;
+ if (nullfd != fd)
+ {
+ if (dup2 (nullfd, fd) < 0)
+ err = 1;
+ close (nullfd);
+ }
+ if (!err)
+ result = orig_freopen (filename, mode, result);
+ }
+ }
return result;
}
diff --git a/m4/freopen.m4 b/m4/freopen.m4
index 8d8e124..727eb08 100644
--- a/m4/freopen.m4
+++ b/m4/freopen.m4
@@ -1,4 +1,4 @@
-# freopen.m4 serial 5
+# freopen.m4 serial 6
dnl Copyright (C) 2007-2016 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -12,6 +12,27 @@ AC_DEFUN([gl_FUNC_FREOPEN],
mingw* | pw* | os2*)
REPLACE_FREOPEN=1
;;
+ *)
+ AC_CACHE_CHECK([whether freopen works on closed fds],
+ [gl_cv_func_freopen_works_on_closed],
+ [AC_RUN_IFELSE(
+ [AC_LANG_PROGRAM(
+ [[#include <stdio.h>
+ #include <unistd.h>
+ ]],
+ [[close (0);
+ return !(freopen ("/dev/null", "r", stdin)
+ && getchar () == EOF
+ && !ferror (stdin) && feof (stdin));]])],
+ [gl_cv_func_freopen_works_on_closed=yes],
+ [gl_cv_func_freopen_works_on_closed=no],
+ [case $host_os in
+ *gnu*) gl_cv_func_freopen_works_on_closed="guessing no" ;;
+ *) gl_cv_func_freopen_works_on_closed="guessing yes";;
+ esac])])
+ case $gl_cv_func_freopen_works_on_closed in
+ *no) REPLACE_FREOPEN=1;;
+ esac
esac
])
diff --git a/modules/freopen b/modules/freopen
index 958cdbb..adb4bbc 100644
--- a/modules/freopen
+++ b/modules/freopen
@@ -6,6 +6,7 @@ lib/freopen.c
m4/freopen.m4
Depends-on:
+fcntl-h [test $REPLACE_FREOPEN = 1]
stdio
largefile
diff --git a/tests/test-freopen.c b/tests/test-freopen.c
index 80453bf..849d7d1 100644
--- a/tests/test-freopen.c
+++ b/tests/test-freopen.c
@@ -33,7 +33,11 @@ main ()
{
const char *filename = "test-freopen.txt";
+ close (STDIN_FILENO);
ASSERT (freopen ("/dev/null", "r", stdin) != NULL);
+ ASSERT (getchar () == EOF);
+ ASSERT (!ferror (stdin));
+ ASSERT (feof (stdin));
#if 0 /* freopen (NULL, ...) is unsupported on most platforms. */
/* Test that freopen() sets errno if someone else closes the stream
>From 7e76f092a00491532b9bbeb8d2655d7e6c313a20 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 26 Nov 2016 14:59:33 -0800
Subject: [PATCH 1/2] build: update gnulib submodule to latest
---
gnulib | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gnulib b/gnulib
index 6b26660..ea96186 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit 6b26660a01125acb394e39ac71635c8df4c110c4
+Subproject commit ea96186d0b25c89aab0de3129fc4bb3f7a5ccd37
--
2.9.3
>From c7c0c8b65981ab2163269ac8869ac763dc9f15c2 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 26 Nov 2016 15:37:43 -0800
Subject: [PATCH 2/2] shuf: test input-closed bug
Problem reported by Alex Ryan (Bug#25029).
* tests/misc/shuf.sh: Test for shuffling with stdin closed.
---
tests/misc/shuf.sh | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tests/misc/shuf.sh b/tests/misc/shuf.sh
index dcc7e82..2e6141b 100755
--- a/tests/misc/shuf.sh
+++ b/tests/misc/shuf.sh
@@ -166,4 +166,9 @@ printf "A\nB\nC\nD\nE\n" | shuf --rep -n0 > exp || framework_failure_
test \! -s exp ||
{ fail=1; echo "--repeat,STDIN,-n0 produced bad output">&2 ; }
+# shuf 8.25 mishandles input if stdin is closed, due to glibc bug#15589.
+# See coreutils bug#25029.
+shuf /dev/null <&- >out || fail=1
+compare /dev/null out || fail=1
+
Exit $fail
--
2.9.3
