bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

fix mkostemp, add pipe2-safer


From: Eric Blake
Subject: fix mkostemp, add pipe2-safer
Date: Sat, 05 Dec 2009 22:20:37 -0700
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.23) Gecko/20090812 Thunderbird/2.0.0.23 Mnenhy/0.7.6.666

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I'm currently testing the following series.  Any thoughts or review on this?

Eric Blake (5):
      cloexec: add dup_cloexec
Rather than having dup_noinherit, specific to w32spawn.h, it seems better
to have a version also ported to Unix.  Eventually, when I get around to
implementing a partial rpl_fcntl, this will allow F_DUPFD_CLOEXEC for
systems that lack it.

      test-dup2: enhance test
Make sure that dup2 of a cloexec fd creates an inheritable fd.

      unistd-safer: allow preservation of cloexec status via flag
Create fd_safer_flag; the counterpart to fd_safer that will honor
O_CLOEXEC when it is defined.  For now, that's pretty much only newer
Linux kernels and mingw, but other platforms will slowly add POSIX 2008
compliance, and we are making progress towards having gnulib be able to
provide an O_CLOEXEC emulation.

      stdlib-safer: preserve cloexec flag for mkostemp[s]
Fix a bug in mkostemp_safer, where it was not preserving O_CLOEXEC status.

      pipe2-safer: new module
Add pipe2_safer, and use it in the pipe module.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksbPyUACgkQ84KuGfSFAYDJqACbBI6FgK43WYFdAcd2gAYBlE2i
/AIAn37qLx/KHULwJc555duqljnpqmIE
=KLW4
-----END PGP SIGNATURE-----
>From d8ccc13ba3ba05654f9be09bba5492cc8da30c3b Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 4 Dec 2009 22:10:44 -0700
Subject: [PATCH 1/5] cloexec: add dup_cloexec

This is needed to enforce correct semantics of mkostemp_safer.
Meanwhile, it is one step closer to providing O_CLOEXEC support
to open, as well as implementing portions of fcntl for mingw.

* lib/cloexec.h (dup_cloexec): New prototype.  Add copyright
header and comments.
* lib/cloexec.c (set_cloexec_flag): Add comments.
(dup_cloexec): New function, with mingw implementation borrowed
from...
* lib/w32spawn.h (dup_noinherit): ...here.
* modules/execute (Depends-on): Add cloexec.
* modules/pipe (Depends-on): Likewise.
* modules/cloexec (Depends-on): Add dup2.
* modules/cloexec-tests (Files): New file.
* tests/test-cloexec.c: Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog             |   13 +++++
 lib/cloexec.c         |  111 ++++++++++++++++++++++++++++++++++++++++++--
 lib/cloexec.h         |   36 ++++++++++++++
 lib/w32spawn.h        |   28 ++----------
 modules/cloexec       |    1 +
 modules/cloexec-tests |   10 ++++
 modules/execute       |    1 +
 modules/pipe          |    1 +
 tests/test-cloexec.c  |  122 +++++++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 293 insertions(+), 30 deletions(-)
 create mode 100644 modules/cloexec-tests
 create mode 100644 tests/test-cloexec.c

diff --git a/ChangeLog b/ChangeLog
index c3d6d73..f0c2abd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
 2009-12-05  Eric Blake  <address@hidden>

+       cloexec: add dup_cloexec
+       * lib/cloexec.h (dup_cloexec): New prototype.  Add copyright
+       header and comments.
+       * lib/cloexec.c (set_cloexec_flag): Add comments.
+       (dup_cloexec): New function, with mingw implementation borrowed
+       from...
+       * lib/w32spawn.h (dup_noinherit): ...here.
+       * modules/execute (Depends-on): Add cloexec.
+       * modules/pipe (Depends-on): Likewise.
+       * modules/cloexec (Depends-on): Add dup2.
+       * modules/cloexec-tests (Files): New file.
+       * tests/test-cloexec.c: Likewise.
+
        test-xalloc-die: fix test for mingw
        * modules/xalloc-die-tests (Files): Add tests/init.sh.
        * tests/test-xalloc-die.sh: Rewrite to use init.sh.  Strip
diff --git a/lib/cloexec.c b/lib/cloexec.c
index 0fb227c..b862cc6 100644
--- a/lib/cloexec.c
+++ b/lib/cloexec.c
@@ -1,6 +1,7 @@
 /* closexec.c - set or clear the close-on-exec descriptor flag

-   Copyright (C) 1991, 2004, 2005, 2006 Free Software Foundation, Inc.
+   Copyright (C) 1991, 2004, 2005, 2006, 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
@@ -21,12 +22,27 @@

 #include "cloexec.h"

-#include <unistd.h>
+#include <errno.h>
 #include <fcntl.h>
+#include <unistd.h>
+
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+/* Native Woe32 API.  */
+# define WIN32_LEAN_AND_MEAN
+# include <windows.h>
+# include <io.h>
+#endif
+

 /* Set the `FD_CLOEXEC' flag of DESC if VALUE is true,
    or clear the flag if VALUE is false.
-   Return 0 on success, or -1 on error with `errno' set. */
+   Return 0 on success, or -1 on error with `errno' set.
+
+   Note that on MingW, this function does NOT protect DESC from being
+   inherited into spawned children.  Instead, either use dup_cloexec
+   followed by closing the original DESC, or use interfaces such as
+   open or pipe2 that accept flags like O_CLOEXEC to create DESC
+   non-inheritable in the first place.  */

 int
 set_cloexec_flag (int desc, bool value)
@@ -40,15 +56,98 @@ set_cloexec_flag (int desc, bool value)
       int newflags = (value ? flags | FD_CLOEXEC : flags & ~FD_CLOEXEC);

       if (flags == newflags
-         || fcntl (desc, F_SETFD, newflags) != -1)
-       return 0;
+          || fcntl (desc, F_SETFD, newflags) != -1)
+        return 0;
     }

   return -1;

 #else

-  return 0;
+  if (desc < 0)
+    {
+      errno = EBADF;
+      return -1;
+    }
+  return dup2 (desc, desc) == desc ? 0 : -1;

 #endif
 }
+
+
+/* Duplicates a file handle FD, while marking the copy to be closed
+   prior to exec or spawn.  Returns -1 and sets errno if FD could not
+   be duplicated.  */
+
+int dup_cloexec (int fd)
+{
+  int nfd;
+
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+
+  /* Native Woe32 API.  */
+  HANDLE curr_process = GetCurrentProcess ();
+  HANDLE old_handle = (HANDLE) _get_osfhandle (fd);
+  HANDLE new_handle;
+
+  if (old_handle == INVALID_HANDLE_VALUE)
+    {
+      /* fd is closed, or is open to no handle at all.
+         We cannot duplicate fd in this case, because _open_osfhandle
+         fails for an INVALID_HANDLE_VALUE argument.  */
+      errno = EBADF;
+      return -1;
+    }
+
+  if (!DuplicateHandle (curr_process,               /* SourceProcessHandle */
+                        old_handle,                 /* SourceHandle */
+                        curr_process,               /* TargetProcessHandle */
+                        (PHANDLE) &new_handle,      /* TargetHandle */
+                        (DWORD) 0,                  /* DesiredAccess */
+                        FALSE,                      /* InheritHandle */
+                        DUPLICATE_SAME_ACCESS))     /* Options */
+    {
+      errno = EMFILE;
+      return -1;
+    }
+
+  nfd = _open_osfhandle ((long) new_handle, O_BINARY | O_NOINHERIT);
+  if (nfd < 0)
+    {
+      CloseHandle (new_handle);
+      errno = EMFILE;
+      return -1;
+    }
+
+#  if REPLACE_FCHDIR
+  if (0 <= nfd)
+    result = _gl_register_dup (fd, nfd);
+#  endif
+  return nfd;
+
+#else /* !_WIN32 */
+
+  /* Unix API.  */
+
+# ifdef F_DUPFD_CLOEXEC
+  nfd = fcntl (fd, F_DUPFD_CLOEXEC, 0);
+#  if REPLACE_FCHDIR
+  if (0 <= nfd)
+    result = _gl_register_dup (fd, nfd);
+#  endif
+
+# else /* !F_DUPFD_CLOEXEC */
+  nfd = dup (fd);
+  if (0 <= nfd && set_cloexec_flag (nfd, true))
+    {
+      int saved_errno = errno;
+      close (nfd);
+      nfd = -1;
+      errno = saved_errno;
+    }
+# endif /* !F_DUPFD_CLOEXEC */
+
+  return nfd;
+
+#endif /* !_WIN32 */
+}
diff --git a/lib/cloexec.h b/lib/cloexec.h
index c25921d..7333452 100644
--- a/lib/cloexec.h
+++ b/lib/cloexec.h
@@ -1,2 +1,38 @@
+/* closexec.c - set or clear the close-on-exec descriptor flag
+
+   Copyright (C) 2004, 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/>.
+
+*/
+
 #include <stdbool.h>
+
+/* Set the `FD_CLOEXEC' flag of DESC if VALUE is true,
+   or clear the flag if VALUE is false.
+   Return 0 on success, or -1 on error with `errno' set.
+
+   Note that on MingW, this function does NOT protect DESC from being
+   inherited into spawned children.  Instead, either use dup_cloexec
+   followed by closing the original DESC, or use interfaces such as
+   open or pipe2 that accept flags like O_CLOEXEC to create DESC
+   non-inheritable in the first place.  */
+
 int set_cloexec_flag (int desc, bool value);
+
+/* Duplicates a file handle FD, while marking the copy to be closed
+   prior to exec or spawn.  Returns -1 and sets errno if FD could not
+   be duplicated.  */
+
+int dup_cloexec (int fd);
diff --git a/lib/w32spawn.h b/lib/w32spawn.h
index 375d0cb..693b119 100644
--- a/lib/w32spawn.h
+++ b/lib/w32spawn.h
@@ -27,6 +27,7 @@
 #include <unistd.h>
 #include <errno.h>

+#include "cloexec.h"
 #include "xalloc.h"

 /* Duplicates a file handle, making the copy uninheritable.
@@ -34,32 +35,11 @@
 static int
 dup_noinherit (int fd)
 {
-  HANDLE curr_process = GetCurrentProcess ();
-  HANDLE old_handle = (HANDLE) _get_osfhandle (fd);
-  HANDLE new_handle;
-  int nfd;
-
-  if (old_handle == INVALID_HANDLE_VALUE)
-    /* fd is closed, or is open to no handle at all.
-       We cannot duplicate fd in this case, because _open_osfhandle fails for
-       an INVALID_HANDLE_VALUE argument.  */
-    return -1;
-
-  if (!DuplicateHandle (curr_process,              /* SourceProcessHandle */
-                       old_handle,                 /* SourceHandle */
-                       curr_process,               /* TargetProcessHandle */
-                       (PHANDLE) &new_handle,      /* TargetHandle */
-                       (DWORD) 0,                  /* DesiredAccess */
-                       FALSE,                      /* InheritHandle */
-                       DUPLICATE_SAME_ACCESS))     /* Options */
-    error (EXIT_FAILURE, 0, _("DuplicateHandle failed with error code 0x%08x"),
-          (unsigned int) GetLastError ());
-
-  nfd = _open_osfhandle ((long) new_handle, O_BINARY | O_NOINHERIT);
-  if (nfd < 0)
+  fd = dup_cloexec (fd);
+  if (fd < 0 && errno == EMFILE)
     error (EXIT_FAILURE, errno, _("_open_osfhandle failed"));

-  return nfd;
+  return fd;
 }

 /* Returns a file descriptor equivalent to FD, except that the resulting file
diff --git a/modules/cloexec b/modules/cloexec
index 60c04c5..7e41fec 100644
--- a/modules/cloexec
+++ b/modules/cloexec
@@ -7,6 +7,7 @@ lib/cloexec.h
 m4/cloexec.m4

 Depends-on:
+dup2
 stdbool

 configure.ac:
diff --git a/modules/cloexec-tests b/modules/cloexec-tests
new file mode 100644
index 0000000..38c304c
--- /dev/null
+++ b/modules/cloexec-tests
@@ -0,0 +1,10 @@
+Files:
+tests/test-cloexec.c
+
+Depends-on:
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-cloexec
+check_PROGRAMS += test-cloexec
diff --git a/modules/execute b/modules/execute
index 88c4fa4..755e3e5 100644
--- a/modules/execute
+++ b/modules/execute
@@ -8,6 +8,7 @@ lib/w32spawn.h
 m4/execute.m4

 Depends-on:
+cloexec
 dup2
 error
 exit
diff --git a/modules/pipe b/modules/pipe
index bb1ebee..1d68eeb 100644
--- a/modules/pipe
+++ b/modules/pipe
@@ -8,6 +8,7 @@ lib/w32spawn.h
 m4/pipe.m4

 Depends-on:
+cloexec
 dup2
 environ
 error
diff --git a/tests/test-cloexec.c b/tests/test-cloexec.c
new file mode 100644
index 0000000..8df733a
--- /dev/null
+++ b/tests/test-cloexec.c
@@ -0,0 +1,122 @@
+/* Test duplicating non-inheritable file descriptors.
+   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 <address@hidden>, 2009.  */
+
+#include <config.h>
+
+#include "cloexec.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+/* Get declarations of the Win32 API functions.  */
+# define WIN32_LEAN_AND_MEAN
+# include <windows.h>
+#endif
+
+#define ASSERT(expr) \
+  do                                                                         \
+    {                                                                        \
+      if (!(expr))                                                           \
+        {                                                                    \
+          fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
+          fflush (stderr);                                                   \
+          abort ();                                                          \
+        }                                                                    \
+    }                                                                        \
+  while (0)
+
+/* Return non-zero if FD is open and inheritable across exec/spawn.  */
+static int
+is_inheritable (int fd)
+{
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+  /* On Win32, the initial state of unassigned standard file
+     descriptors is that they are open but point to an
+     INVALID_HANDLE_VALUE, and there is no fcntl.  */
+  HANDLE h = (HANDLE) _get_osfhandle (fd);
+  DWORD flags;
+  if (h == INVALID_HANDLE_VALUE || GetHandleInformation (h, &flags) == 0)
+    return 0;
+  return (flags & HANDLE_FLAG_INHERIT) != 0;
+#else
+# ifndef F_GETFD
+#  error Please port fcntl to your platform
+# endif
+  int i = fcntl (fd, F_GETFD);
+  return 0 <= i && (i & FD_CLOEXEC) == 0;
+#endif
+}
+
+int
+main (void)
+{
+  const char *file = "test-cloexec.tmp";
+  int fd = creat (file, 0600);
+  int fd2;
+
+  /* Assume std descriptors were provided by invoker.  */
+  ASSERT (STDERR_FILENO < fd);
+  ASSERT (is_inheritable (fd));
+
+  /* Normal use of set_cloexec_flag.  */
+  ASSERT (set_cloexec_flag (fd, true) == 0);
+#if !((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)
+  ASSERT (!is_inheritable (fd));
+#endif
+  ASSERT (set_cloexec_flag (fd, false) == 0);
+  ASSERT (is_inheritable (fd));
+
+  /* Normal use of dup_cloexec.  */
+  fd2 = dup_cloexec (fd);
+  ASSERT (fd < fd2);
+  ASSERT (!is_inheritable (fd2));
+  ASSERT (close (fd) == 0);
+  ASSERT (dup_cloexec (fd2) == fd);
+  ASSERT (!is_inheritable (fd));
+  ASSERT (close (fd2) == 0);
+
+  /* Test error handling.  */
+  errno = 0;
+  ASSERT (set_cloexec_flag (-1, false) == -1);
+  ASSERT (errno == EBADF);
+  errno = 0;
+  ASSERT (set_cloexec_flag (10000000, false) == -1);
+  ASSERT (errno == EBADF);
+  errno = 0;
+  ASSERT (set_cloexec_flag (fd2, false) == -1);
+  ASSERT (errno == EBADF);
+  errno = 0;
+  ASSERT (dup_cloexec (-1) == -1);
+  ASSERT (errno == EBADF);
+  errno = 0;
+  ASSERT (dup_cloexec (10000000) == -1);
+  ASSERT (errno == EBADF);
+  errno = 0;
+  ASSERT (dup_cloexec (fd2) == -1);
+  ASSERT (errno == EBADF);
+
+  /* Clean up.  */
+  ASSERT (close (fd) == 0);
+  ASSERT (unlink (file) == 0);
+
+  return 0;
+}
-- 
1.6.5.rc1


>From 173359dbe40ac4381088060b0f26e66529fa0f29 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Sat, 5 Dec 2009 06:19:01 -0700
Subject: [PATCH 2/5] test-dup2: enhance test

Ensure that dup2(cloexec_fd, target) returns an inheritable fd.

* modules/dup2-tests (Depends-on): Add cloexec.
* tests/test-dup2.c (main): Enhance test.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog          |    4 ++++
 modules/dup2-tests |    1 +
 tests/test-dup2.c  |   50 ++++++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f0c2abd..0c1c870 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2009-12-05  Eric Blake  <address@hidden>

+       test-dup2: enhance test
+       * modules/dup2-tests (Depends-on): Add cloexec.
+       * tests/test-dup2.c (main): Enhance test.
+
        cloexec: add dup_cloexec
        * lib/cloexec.h (dup_cloexec): New prototype.  Add copyright
        header and comments.
diff --git a/modules/dup2-tests b/modules/dup2-tests
index f11f138..0fb913a 100644
--- a/modules/dup2-tests
+++ b/modules/dup2-tests
@@ -2,6 +2,7 @@ Files:
 tests/test-dup2.c

 Depends-on:
+cloexec
 open

 configure.ac:
diff --git a/tests/test-dup2.c b/tests/test-dup2.c
index 32c354d..005160f 100644
--- a/tests/test-dup2.c
+++ b/tests/test-dup2.c
@@ -25,6 +25,8 @@
 #include <stdio.h>
 #include <stdlib.h>

+#include "cloexec.h"
+
 #if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
 /* Get declarations of the Win32 API functions.  */
 # define WIN32_LEAN_AND_MEAN
@@ -32,15 +34,15 @@
 #endif

 #define ASSERT(expr) \
-  do                                                                        \
-    {                                                                       \
-      if (!(expr))                                                          \
-        {                                                                   \
+  do                                                                         \
+    {                                                                        \
+      if (!(expr))                                                           \
+        {                                                                    \
           fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
-          fflush (stderr);                                                  \
-          abort ();                                                         \
-        }                                                                   \
-    }                                                                       \
+          fflush (stderr);                                                   \
+          abort ();                                                          \
+        }                                                                    \
+    }                                                                        \
   while (0)

 /* Return non-zero if FD is open.  */
@@ -60,6 +62,28 @@ is_open (int fd)
 #endif
 }

+/* Return non-zero if FD is open and inheritable across exec/spawn.  */
+static int
+is_inheritable (int fd)
+{
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+  /* On Win32, the initial state of unassigned standard file
+     descriptors is that they are open but point to an
+     INVALID_HANDLE_VALUE, and there is no fcntl.  */
+  HANDLE h = (HANDLE) _get_osfhandle (fd);
+  DWORD flags;
+  if (h == INVALID_HANDLE_VALUE || GetHandleInformation (h, &flags) == 0)
+    return 0;
+  return (flags & HANDLE_FLAG_INHERIT) != 0;
+#else
+# ifndef F_GETFD
+#  error Please port fcntl to your platform
+# endif
+  int i = fcntl (fd, F_GETFD);
+  return 0 <= i && (i & FD_CLOEXEC) == 0;
+#endif
+}
+
 int
 main (void)
 {
@@ -125,8 +149,18 @@ main (void)
   ASSERT (read (fd, buffer, 1) == 1);
   ASSERT (*buffer == '2');

+  /* Any new fd created by dup2 must not be cloexec.  */
+  ASSERT (close (fd + 2) == 0);
+  ASSERT (dup_cloexec (fd) == fd + 1);
+  ASSERT (!is_inheritable (fd + 1));
+  ASSERT (dup2 (fd + 1, fd + 1) == fd + 1);
+  ASSERT (!is_inheritable (fd + 1));
+  ASSERT (dup2 (fd + 1, fd + 2) == fd + 2);
+  ASSERT (is_inheritable (fd + 2));
+
   /* Clean up.  */
   ASSERT (close (fd + 2) == 0);
+  ASSERT (close (fd + 1) == 0);
   ASSERT (close (fd) == 0);
   ASSERT (unlink (file) == 0);

-- 
1.6.5.rc1


>From 14449a1dacca44c4f933d5fc207c7fa56e6c0fb7 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Sat, 5 Dec 2009 06:36:33 -0700
Subject: [PATCH 3/5] unistd-safer: allow preservation of cloexec status via flag

If cloexec is in use, allow the ability to preserve cloexec
flag across *_safer functions.

* lib/unistd-safer.h (dup_safer_flag, fd_safer_flag): New
prototypes.
* lib/dup-safer.c (dup_safer_flag): New function.
* lib/fd-safer.c (fd_safer_flag): Likewise.
* modules/cloexec (configure.ac): Set witness.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog          |    7 +++++++
 lib/dup-safer.c    |   32 +++++++++++++++++++++++++++++++-
 lib/fd-safer.c     |   29 +++++++++++++++++++++++++++++
 lib/unistd-safer.h |    9 +++++++--
 modules/cloexec    |    1 +
 5 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0c1c870..20535a0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2009-12-05  Eric Blake  <address@hidden>

+       unistd-safer: allow preservation of cloexec status via flag
+       * lib/unistd-safer.h (dup_safer_flag, fd_safer_flag): New
+       prototypes.
+       * lib/dup-safer.c (dup_safer_flag): New function.
+       * lib/fd-safer.c (fd_safer_flag): Likewise.
+       * modules/cloexec (configure.ac): Set witness.
+
        test-dup2: enhance test
        * modules/dup2-tests (Depends-on): Add cloexec.
        * tests/test-dup2.c (main): Enhance test.
diff --git a/lib/dup-safer.c b/lib/dup-safer.c
index 22b96ba..2af8b6a 100644
--- a/lib/dup-safer.c
+++ b/lib/dup-safer.c
@@ -23,7 +23,6 @@
 #include "unistd-safer.h"

 #include <fcntl.h>
-
 #include <unistd.h>

 /* Like dup, but do not return STDIN_FILENO, STDOUT_FILENO, or
@@ -40,3 +39,34 @@ dup_safer (int fd)
   return fd_safer (dup (fd));
 #endif
 }
+
+#if GNULIB_CLOEXEC
+
+# include "cloexec.h"
+
+# ifndef O_CLOEXEC
+#  define O_CLOEXEC 0
+# endif
+
+/* Like dup, but do not return STDIN_FILENO, STDOUT_FILENO, or
+   STDERR_FILENO.  If FLAG contains O_CLOEXEC, behave like
+   fcntl(F_DUPFD_CLOEXEC) rather than fcntl(F_DUPFD).  */
+
+int
+dup_safer_flag (int fd, int flag)
+{
+  if (flag & O_CLOEXEC)
+    {
+# if defined F_DUPFD_CLOEXEC && !REPLACE_FCHDIR
+      return fcntl (fd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1);
+# else
+      /* fd_safer_flag calls us back, but eventually the recursion
+         unwinds and does the right thing.  */
+      fd = dup_cloexec (fd);
+      return fd_safer_flag (fd, flag);
+# endif
+    }
+  return dup_safer (fd);
+}
+
+#endif
diff --git a/lib/fd-safer.c b/lib/fd-safer.c
index fb99001..d2e1309 100644
--- a/lib/fd-safer.c
+++ b/lib/fd-safer.c
@@ -48,3 +48,32 @@ fd_safer (int fd)

   return fd;
 }
+
+#if GNULIB_CLOEXEC
+
+/* Return FD, unless FD would be a copy of standard input, output, or
+   error; in that case, return a duplicate of FD, closing FD.  If FLAG
+   contains O_CLOEXEC, the returned FD will have close-on-exec
+   semantics.  On failure to duplicate, close FD, set errno, and
+   return -1.  Preserve errno if FD is negative, so that the caller
+   can always inspect errno when the returned value is negative.
+
+   This function is usefully wrapped around functions that return file
+   descriptors, e.g., fd_safer_flag (open ("file", O_RDONLY | flag), flag).  */
+
+int
+fd_safer_flag (int fd, int flag)
+{
+  if (STDIN_FILENO <= fd && fd <= STDERR_FILENO)
+    {
+      int f = dup_safer_flag (fd, flag);
+      int e = errno;
+      close (fd);
+      errno = e;
+      fd = f;
+    }
+
+  return fd;
+}
+
+#endif /* GNULIB_CLOEXEC */
diff --git a/lib/unistd-safer.h b/lib/unistd-safer.h
index 033e857..af7d4ea 100644
--- a/lib/unistd-safer.h
+++ b/lib/unistd-safer.h
@@ -1,6 +1,6 @@
 /* Invoke unistd-like functions, but avoid some glitches.

-   Copyright (C) 2001, 2003, 2005 Free Software Foundation, Inc.
+   Copyright (C) 2001, 2003, 2005, 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
@@ -15,8 +15,13 @@
    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 Paul Eggert.  */
+/* Written by Paul Eggert and Eric Blake.  */

 int dup_safer (int);
 int fd_safer (int);
 int pipe_safer (int[2]);
+
+#if GNULIB_CLOEXEC
+int dup_safer_flag (int, int);
+int fd_safer_flag (int, int);
+#endif
diff --git a/modules/cloexec b/modules/cloexec
index 7e41fec..7d987de 100644
--- a/modules/cloexec
+++ b/modules/cloexec
@@ -12,6 +12,7 @@ stdbool

 configure.ac:
 gl_CLOEXEC
+gl_MODULE_INDICATOR([cloexec])

 Makefile.am:

-- 
1.6.5.rc1


>From f2fdd542de54c78c89fd8b50b76921cf31fdb518 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 16 Nov 2009 16:09:42 -0700
Subject: [PATCH 4/5] stdlib-safer: preserve cloexec flag for mkostemp[s]

mkostemp_safer(templ,O_CLOEXEC) did not always guarantee cloexec.

* lib/mkstemp-safer.c (mkostemp_safer, mkostemps_safer): Use new
fd_safer_flag.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog           |    4 ++++
 lib/mkstemp-safer.c |    4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 20535a0..21c86be 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2009-12-05  Eric Blake  <address@hidden>

+       stdlib-safer: preserve cloexec flag for mkostemp[s]
+       * lib/mkstemp-safer.c (mkostemp_safer, mkostemps_safer): Use new
+       fd_safer_flag.
+
        unistd-safer: allow preservation of cloexec status via flag
        * lib/unistd-safer.h (dup_safer_flag, fd_safer_flag): New
        prototypes.
diff --git a/lib/mkstemp-safer.c b/lib/mkstemp-safer.c
index 95d315b..ee242f3 100644
--- a/lib/mkstemp-safer.c
+++ b/lib/mkstemp-safer.c
@@ -39,7 +39,7 @@ mkstemp_safer (char *templ)
 int
 mkostemp_safer (char *templ, int flags)
 {
-  return fd_safer (mkostemp (templ, flags));
+  return fd_safer_flag (mkostemp (templ, flags), flags);
 }
 #endif

@@ -49,7 +49,7 @@ mkostemp_safer (char *templ, int flags)
 int
 mkostemps_safer (char *templ, int suffixlen, int flags)
 {
-  return fd_safer (mkostemps (templ, suffixlen, flags));
+  return fd_safer_flag (mkostemps (templ, suffixlen, flags), flags);
 }
 #endif

-- 
1.6.5.rc1


>From 6ed67a15a265e213d2cbaac25ccc995fddf73ac6 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Sat, 5 Dec 2009 06:39:09 -0700
Subject: [PATCH 5/5] pipe2-safer: new module

pipe2 deserves a *_safer variant.  It also makes the code in
pipe.c look simpler.

* modules/pipe2-safer: New file.
* lib/unistd-safer.h (pipe2_safer): New prototype.
* lib/unistd--.h (pipe2): New wrapper.
* lib/pipe-safer.c (pipe2_safer): New function.
* modules/pipe (Depends-on): Add pipe2-safer.
* lib/pipe.c (create_pipe) [WIN32]: Let pipe2_safer do the work.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog           |    8 ++++++++
 lib/pipe-safer.c    |   50 +++++++++++++++++++++++++++++++++++++++-----------
 lib/pipe.c          |    8 ++------
 lib/unistd--.h      |    7 ++++++-
 lib/unistd-safer.h  |    4 ++++
 modules/pipe        |    1 +
 modules/pipe2-safer |   24 ++++++++++++++++++++++++
 7 files changed, 84 insertions(+), 18 deletions(-)
 create mode 100644 modules/pipe2-safer

diff --git a/ChangeLog b/ChangeLog
index 21c86be..90d6eb0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-12-05  Eric Blake  <address@hidden>

+       pipe2-safer: new module
+       * modules/pipe2-safer: New file.
+       * lib/unistd-safer.h (pipe2_safer): New prototype.
+       * lib/unistd--.h (pipe2): New wrapper.
+       * lib/pipe-safer.c (pipe2_safer): New function.
+       * modules/pipe (Depends-on): Add pipe2-safer.
+       * lib/pipe.c (create_pipe) [WIN32]: Let pipe2_safer do the work.
+
        stdlib-safer: preserve cloexec flag for mkostemp[s]
        * lib/mkstemp-safer.c (mkostemp_safer, mkostemps_safer): Use new
        fd_safer_flag.
diff --git a/lib/pipe-safer.c b/lib/pipe-safer.c
index 0fc6850..fda5c52 100644
--- a/lib/pipe-safer.c
+++ b/lib/pipe-safer.c
@@ -1,5 +1,5 @@
 /* Invoke pipe, but avoid some glitches.
-   Copyright (C) 2005, 2006 Free Software Foundation, Inc.
+   Copyright (C) 2005, 2006, 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
@@ -35,16 +35,16 @@ pipe_safer (int fd[2])
     {
       int i;
       for (i = 0; i < 2; i++)
-       {
-         fd[i] = fd_safer (fd[i]);
-         if (fd[i] < 0)
-           {
-             int e = errno;
-             close (fd[1 - i]);
-             errno = e;
-             return -1;
-           }
-       }
+        {
+          fd[i] = fd_safer (fd[i]);
+          if (fd[i] < 0)
+            {
+              int e = errno;
+              close (fd[1 - i]);
+              errno = e;
+              return -1;
+            }
+        }

       return 0;
     }
@@ -54,3 +54,31 @@ pipe_safer (int fd[2])

   return -1;
 }
+
+#if GNULIB_PIPE2_SAFER
+/* Like pipe2, but ensure that neither of the file descriptors is
+   STDIN_FILENO, STDOUT_FILENO, or STDERR_FILENO.  */
+
+int
+pipe2_safer (int fd[2], int flags)
+{
+  if (pipe2 (fd, flags) == 0)
+    {
+      int i;
+      for (i = 0; i < 2; i++)
+        {
+          fd[i] = fd_safer_flag (fd[i], flags);
+          if (fd[i] < 0)
+            {
+              int e = errno;
+              close (fd[1 - i]);
+              errno = e;
+              return -1;
+            }
+        }
+
+      return 0;
+    }
+  return -1;
+}
+#endif /* GNULIB_PIPE2 */
diff --git a/lib/pipe.c b/lib/pipe.c
index d17c9bf..c3db1b5 100644
--- a/lib/pipe.c
+++ b/lib/pipe.c
@@ -133,14 +133,10 @@ create_pipe (const char *progname,
   prog_argv = prepare_spawn (prog_argv);

   if (pipe_stdout)
-    if (pipe2 (ifd, O_BINARY | O_NOINHERIT) < 0
-       || (ifd[0] = fd_safer_noinherit (ifd[0])) < 0
-       || (ifd[1] = fd_safer_noinherit (ifd[1])) < 0)
+    if (pipe2_safer (ifd, O_BINARY | O_CLOEXEC) < 0)
       error (EXIT_FAILURE, errno, _("cannot create pipe"));
   if (pipe_stdin)
-    if (pipe2 (ofd, O_BINARY | O_NOINHERIT) < 0
-       || (ofd[0] = fd_safer_noinherit (ofd[0])) < 0
-       || (ofd[1] = fd_safer_noinherit (ofd[1])) < 0)
+    if (pipe2_safer (ofd, O_BINARY | O_CLOEXEC) < 0)
       error (EXIT_FAILURE, errno, _("cannot create pipe"));
 /* Data flow diagram:
  *
diff --git a/lib/unistd--.h b/lib/unistd--.h
index 1a7fd78..6f1ebaf 100644
--- a/lib/unistd--.h
+++ b/lib/unistd--.h
@@ -1,6 +1,6 @@
 /* Like unistd.h, but redefine some names to avoid glitches.

-   Copyright (C) 2005 Free Software Foundation, Inc.
+   Copyright (C) 2005, 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
@@ -25,3 +25,8 @@

 #undef pipe
 #define pipe pipe_safer
+
+#if GNULIB_PIPE2_SAFER
+# undef pipe2
+# define pipe2 pipe2_safer
+#endif
diff --git a/lib/unistd-safer.h b/lib/unistd-safer.h
index af7d4ea..70cc699 100644
--- a/lib/unistd-safer.h
+++ b/lib/unistd-safer.h
@@ -25,3 +25,7 @@ int pipe_safer (int[2]);
 int dup_safer_flag (int, int);
 int fd_safer_flag (int, int);
 #endif
+
+#if GNULIB_PIPE2_SAFER
+int pipe2_safer (int[2], int);
+#endif
diff --git a/modules/pipe b/modules/pipe
index 1d68eeb..922bda1 100644
--- a/modules/pipe
+++ b/modules/pipe
@@ -17,6 +17,7 @@ fatal-signal
 gettext-h
 open
 pipe2
+pipe2-safer
 spawn
 posix_spawnp
 posix_spawn_file_actions_init
diff --git a/modules/pipe2-safer b/modules/pipe2-safer
new file mode 100644
index 0000000..eb59487
--- /dev/null
+++ b/modules/pipe2-safer
@@ -0,0 +1,24 @@
+Description:
+pipe2_safer() function: create a pipe, with specific opening flags,
+without clobbering std{in,out,err}.
+
+Files:
+
+Depends-on:
+cloexec
+pipe2
+unistd-safer
+
+configure.ac:
+gl_MODULE_INDICATOR([pipe2-safer])
+
+Makefile.am:
+
+Include:
+"unistd-safer.h"
+
+License:
+GPL
+
+Maintainer:
+Eric Blake
-- 
1.6.5.rc1


reply via email to

[Prev in Thread] Current Thread [Next in Thread]