changeset 11848:9e3299dad578

popen: fix cygwin 1.5 bug when stdin closed * doc/posix-functions/popen.texi (popen): Document cygwin bugs. * modules/popen: New file. * modules/popen-tests: Likewise. * tests/test-popen.c: Likewise. * m4/popen.m4: Likewise. * lib/popen.c: Likewise. * lib/stdio.in.h (popen): New declaration. * m4/stdio_h.m4 (gl_STDIO_H_DEFAULTS): Add popen. * modules/stdio (Makefile.am): Likewise. * MODULES.html.sh (systems lacking POSIX:2008): Mention it. Signed-off-by: Eric Blake <ebb9@byu.net>
author Eric Blake <ebb9@byu.net>
date Wed, 19 Aug 2009 07:15:54 -0600
parents 5dd8e8cf05db
children 18b5c404aa04
files ChangeLog MODULES.html.sh doc/posix-functions/popen.texi lib/popen.c lib/stdio.in.h m4/popen.m4 m4/stdio_h.m4 modules/popen modules/popen-tests modules/stdio tests/test-popen.c
diffstat 11 files changed, 302 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2009-08-19  Eric Blake  <ebb9@byu.net>
+
+	popen: fix cygwin 1.5 bug when stdin closed
+	* doc/posix-functions/popen.texi (popen): Document cygwin bugs.
+	* modules/popen: New file.
+	* modules/popen-tests: Likewise.
+	* tests/test-popen.c: Likewise.
+	* m4/popen.m4: Likewise.
+	* lib/popen.c: Likewise.
+	* lib/stdio.in.h (popen): New declaration.
+	* m4/stdio_h.m4 (gl_STDIO_H_DEFAULTS): Add popen.
+	* modules/stdio (Makefile.am): Likewise.
+	* MODULES.html.sh (systems lacking POSIX:2008): Mention it.
+
 2009-08-17  Joel E. Denny  <jdenny@clemson.edu>
 
 	maint.mk: give full control over update-copyright exclusions
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -2290,6 +2290,7 @@
   func_module open
   func_module perror
   func_module poll
+  func_module popen
   func_module posix_spawn
   func_module posix_spawnattr_destroy
   func_module posix_spawnattr_getflags
--- a/doc/posix-functions/popen.texi
+++ b/doc/posix-functions/popen.texi
@@ -4,12 +4,21 @@
 
 POSIX specification: @url{http://www.opengroup.org/onlinepubs/9699919799/functions/popen.html}
 
-Gnulib module: ---
+Gnulib module: popen
 
 Portability problems fixed by Gnulib:
 @itemize
+@item
+Some platforms start the child with closed stdin or stdout if the
+standard descriptors were closed in the parent:
+Cygwin 1.5.x.
 @end itemize
 
 Portability problems not fixed by Gnulib:
 @itemize
+@item
+Some platforms mistakenly set the close-on-exec bit, then if it is
+cleared by the application, the platform then leaks file descriptors
+from earlier @code{popen} calls into subsequent @code{popen} children:
+Cygwin 1.5.x.
 @end itemize
new file mode 100644
--- /dev/null
+++ b/lib/popen.c
@@ -0,0 +1,81 @@
+/* Open a stream to a sub-process.
+   Copyright (C) 2009 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Eric Blake <ebb9@byu.net>, 2009.  */
+
+#include <config.h>
+
+/* Get the original definition of popen.  It might be defined as a macro.  */
+#define __need_FILE
+# include <stdio.h>
+#undef __need_FILE
+
+static inline FILE *
+orig_popen (const char *filename, const char *mode)
+{
+  return popen (filename, mode);
+}
+
+/* Specification.  */
+#include <stdio.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+FILE *
+rpl_popen (const char *filename, const char *mode)
+{
+  /* The mingw popen works fine, and all other platforms have fcntl.
+     The bug of the child clobbering its own file descriptors if stdin
+     or stdout was closed in the parent can be worked around by
+     opening those two fds as close-on-exec to begin with.  */
+  /* Cygwin 1.5.x also has a bug where the popen fd is improperly
+     marked close-on-exec, and if the application undoes this, then
+     the fd leaks into subsequent popen calls.  We could work around
+     this by maintaining a list of all fd's opened by popen, and
+     temporarily marking them cloexec around the real popen call, but
+     we would also have to override pclose, and the bookkeepping seems
+     extreme given that cygwin 1.7 no longer has the bug.  */
+  FILE *result;
+  int cloexec0 = fcntl (STDIN_FILENO, F_GETFD);
+  int cloexec1 = fcntl (STDOUT_FILENO, F_GETFD);
+  int saved_errno;
+
+  if (cloexec0 < 0)
+    {
+      if (open ("/dev/null", O_RDONLY) != STDIN_FILENO
+	  || fcntl (STDIN_FILENO, F_SETFD,
+		    fcntl (STDIN_FILENO, F_GETFD) | FD_CLOEXEC) == -1)
+	abort ();
+    }
+  if (cloexec1 < 0)
+    {
+      if (open ("/dev/null", O_RDONLY) != STDOUT_FILENO
+	  || fcntl (STDOUT_FILENO, F_SETFD,
+		    fcntl (STDOUT_FILENO, F_GETFD) | FD_CLOEXEC) == -1)
+	abort ();
+    }
+  result = orig_popen (filename, mode);
+  saved_errno = errno;
+  if (cloexec0 < 0)
+    close (STDIN_FILENO);
+  if (cloexec1 < 0)
+    close (STDOUT_FILENO);
+  errno = saved_errno;
+  return result;
+}
--- a/lib/stdio.in.h
+++ b/lib/stdio.in.h
@@ -479,6 +479,20 @@
 extern size_t fwrite (const void *ptr, size_t s, size_t n, FILE *stream);
 #endif
 
+#if @GNULIB_POPEN@
+# if @REPLACE_POPEN@
+#  undef popen
+#  define popen rpl_popen
+extern FILE *popen (const char *cmd, const char *mode);
+# endif
+#elif defined GNULIB_POSIXCHECK
+# undef popen
+# define popen(c,m) \
+   (GL_LINK_WARNING ("popen is buggy on some platforms - " \
+                     "use gnulib module popen or pipe for more portability"), \
+    popen (c, m))
+#endif
+
 #if @GNULIB_GETDELIM@
 # if !@HAVE_DECL_GETDELIM@
 /* Read input, up to (and including) the next occurrence of DELIMITER, from
new file mode 100644
--- /dev/null
+++ b/m4/popen.m4
@@ -0,0 +1,34 @@
+# popen.m4 serial 1
+dnl Copyright (C) 2009 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+AC_DEFUN([gl_FUNC_POPEN],
+[
+  AC_REQUIRE([gl_STDIO_H_DEFAULTS])
+  AC_CACHE_CHECK([whether popen works with closed stdin],
+    [gl_cv_func_popen_works],
+    [
+      AC_RUN_IFELSE([AC_LANG_PROGRAM([[#include <stdio.h>
+]], [FILE *child;
+     fclose (stdin);
+     fclose (stdout);
+     child = popen ("echo a", "r");
+     return !(fgetc (child) == 'a' && pclose (child) == 0);
+])], [gl_cv_func_popen_works=yes], [gl_cv_func_popen_works=no],
+     dnl For now, only cygwin 1.5 or older is known to be broken.
+     [gl_cv_func_popen_works='guessing yes'])
+  ])
+  if test "$gl_cv_func_popen_works" = no; then
+    REPLACE_POPEN=1
+    AC_LIBOBJ([popen])
+    gl_PREREQ_POPEN
+  fi
+])
+
+# Prerequisites of lib/popen.c.
+AC_DEFUN([gl_PREREQ_POPEN],
+[
+  AC_REQUIRE([AC_C_INLINE])
+])
--- a/m4/stdio_h.m4
+++ b/m4/stdio_h.m4
@@ -1,4 +1,4 @@
-# stdio_h.m4 serial 16
+# stdio_h.m4 serial 17
 dnl Copyright (C) 2007-2009 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -73,6 +73,7 @@
   GNULIB_FPUTS=0;                AC_SUBST([GNULIB_FPUTS])
   GNULIB_PUTS=0;                 AC_SUBST([GNULIB_PUTS])
   GNULIB_FWRITE=0;               AC_SUBST([GNULIB_FWRITE])
+  GNULIB_POPEN=0;                AC_SUBST([GNULIB_POPEN])
   GNULIB_GETDELIM=0;             AC_SUBST([GNULIB_GETDELIM])
   GNULIB_GETLINE=0;              AC_SUBST([GNULIB_GETLINE])
   GNULIB_PERROR=0;               AC_SUBST([GNULIB_PERROR])
@@ -109,6 +110,7 @@
   REPLACE_FPURGE=0;              AC_SUBST([REPLACE_FPURGE])
   HAVE_DECL_FPURGE=1;            AC_SUBST([HAVE_DECL_FPURGE])
   REPLACE_FCLOSE=0;              AC_SUBST([REPLACE_FCLOSE])
+  REPLACE_POPEN=0;               AC_SUBST([REPLACE_POPEN])
   HAVE_DECL_GETDELIM=1;          AC_SUBST([HAVE_DECL_GETDELIM])
   HAVE_DECL_GETLINE=1;           AC_SUBST([HAVE_DECL_GETLINE])
   REPLACE_GETLINE=0;             AC_SUBST([REPLACE_GETLINE])
new file mode 100644
--- /dev/null
+++ b/modules/popen
@@ -0,0 +1,25 @@
+Description:
+popen() function: open a stream to a shell command.
+
+Files:
+lib/popen.c
+m4/popen.m4
+
+Depends-on:
+open
+stdio
+
+configure.ac:
+gl_FUNC_POPEN
+gl_STDIO_MODULE_INDICATOR([popen])
+
+Makefile.am:
+
+Include:
+<stdio.h>
+
+License:
+LGPL
+
+Maintainer:
+Eric Blake
new file mode 100644
--- /dev/null
+++ b/modules/popen-tests
@@ -0,0 +1,12 @@
+Files:
+tests/test-popen.c
+
+Depends-on:
+dup2
+sys_wait
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-popen
+check_PROGRAMS += test-popen
--- a/modules/stdio
+++ b/modules/stdio
@@ -58,6 +58,7 @@
 	      -e 's|@''GNULIB_FPUTS''@|$(GNULIB_FPUTS)|g' \
 	      -e 's|@''GNULIB_PUTS''@|$(GNULIB_PUTS)|g' \
 	      -e 's|@''GNULIB_FWRITE''@|$(GNULIB_FWRITE)|g' \
+	      -e 's|@''GNULIB_POPEN''@|$(GNULIB_POPEN)|g' \
 	      -e 's|@''GNULIB_GETDELIM''@|$(GNULIB_GETDELIM)|g' \
 	      -e 's|@''GNULIB_GETLINE''@|$(GNULIB_GETLINE)|g' \
 	      -e 's|@''GNULIB_PERROR''@|$(GNULIB_PERROR)|g' \
@@ -91,6 +92,7 @@
 	      -e 's|@''REPLACE_FPURGE''@|$(REPLACE_FPURGE)|g' \
 	      -e 's|@''HAVE_DECL_FPURGE''@|$(HAVE_DECL_FPURGE)|g' \
 	      -e 's|@''REPLACE_FCLOSE''@|$(REPLACE_FCLOSE)|g' \
+	      -e 's|@''REPLACE_POPEN''@|$(REPLACE_POPEN)|g' \
 	      -e 's|@''HAVE_DECL_GETDELIM''@|$(HAVE_DECL_GETDELIM)|g' \
 	      -e 's|@''HAVE_DECL_GETLINE''@|$(HAVE_DECL_GETLINE)|g' \
 	      -e 's|@''REPLACE_GETLINE''@|$(REPLACE_GETLINE)|g' \
new file mode 100644
--- /dev/null
+++ b/tests/test-popen.c
@@ -0,0 +1,106 @@
+/* Test of opening a subcommand stream.
+   Copyright (C) 2009 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Eric Blake <ebb9@byu.net>, 2009.  */
+
+#include <config.h>
+
+/* Specification.  */
+#include <stdio.h>
+
+/* Helpers.  */
+#include <stdlib.h>
+#include <string.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#define ASSERT(expr) \
+  do									     \
+    {									     \
+      if (!(expr))							     \
+        {								     \
+          fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
+          fflush (stderr);						     \
+          abort ();							     \
+        }								     \
+    }									     \
+  while (0)
+
+int
+main (int argc, char **argv)
+{
+  size_t len;
+  char *cmd;
+  int i;
+
+  /* Children - use the pipe.  */
+  if (argc > 1)
+    {
+      if (*argv[1] == 'r') /* Parent is reading, so we write.  */
+	ASSERT (putchar ('c') == 'c');
+      else /* Parent is writing, so we read.  */
+	ASSERT (getchar () == 'p');
+      /* Test that parent can read non-zero status.  */
+      return 42;
+    }
+
+  /* Parent - create read and write child, once under normal
+     circumstances and once with stdin and stdout closed.  */
+  len = strlen (argv[0]);
+  cmd = malloc (len + 3); /* Adding " r" and NUL.  */
+  ASSERT (cmd);
+  /* We count on argv[0] not containing any shell metacharacters.  */
+  strcpy (cmd, argv[0]);
+  cmd[len] = ' ';
+  cmd[len + 2] = '\0';
+  for (i = 0; i < 2; i++)
+    {
+      FILE *child;
+      int status;
+
+      if (i)
+	{
+	  ASSERT (fclose (stdin) == 0);
+	  ASSERT (fclose (stdout) == 0);
+	}
+
+      cmd[len + 1] = 'r';
+      ASSERT (child = popen (cmd, "r"));
+      ASSERT (fgetc (child) == 'c');
+      status = pclose (child);
+      ASSERT (WIFEXITED (status));
+      ASSERT (WEXITSTATUS (status) == 42);
+      if (i)
+	{
+	  ASSERT (dup2 (STDIN_FILENO, STDIN_FILENO) == -1);
+	  ASSERT (dup2 (STDOUT_FILENO, STDOUT_FILENO) == -1);
+	}
+
+      cmd[len + 1] = 'w';
+      ASSERT (child = popen (cmd, "w"));
+      ASSERT (fputc ('p', child) == 'p');
+      status = pclose (child);
+      ASSERT (WIFEXITED (status));
+      ASSERT (WEXITSTATUS (status) == 42);
+      if (i)
+	{
+	  ASSERT (dup2 (STDIN_FILENO, STDIN_FILENO) == -1);
+	  ASSERT (dup2 (STDOUT_FILENO, STDOUT_FILENO) == -1);
+	}
+    }
+  free (cmd);
+  return 0;
+}