changeset 12457:2a3833485e1c

fcntl: use to simplify other modules Let fcntl do the work, instead of copying code into other modules. * modules/cloexec (Depends-on): Add fcntl. * modules/fchdir (Depends-on): Likewise. * modules/fd-safer-flag (Depends-on): Likewise. * modules/unistd-safer (Depends-on): Likewise. * modules/dup3 (configure.ac): Set module indicator. * m4/fchdir.m4 (gl_FUNC_FCHDIR): Replace fcntl if fchdir is missing. * lib/fchdir.c (_gl_register_dup): Fix comment. * lib/cloexec.c (dup_cloexec): Simplify, by relying on fcntl. * lib/dup-safer.c (dup_safer): Likewise. * lib/dup-safer-flag.c (dup_safer_flag): Likewise. * lib/dup3.c (dup3): Likewise. * tests/test-fchdir.c (main): Enhance test. Fixes a dup_cloexec bug reported by Ondřej Vašík. Signed-off-by: Eric Blake <ebb9@byu.net>
author Eric Blake <ebb9@byu.net>
date Wed, 16 Dec 2009 10:07:13 -0700
parents f3aceada3c52
children fd1286c1d1b9
files ChangeLog lib/cloexec.c lib/dup-safer-flag.c lib/dup-safer.c lib/dup3.c lib/fchdir.c m4/fchdir.m4 modules/cloexec modules/dup3 modules/fchdir modules/fd-safer-flag modules/unistd-safer tests/test-fchdir.c
diffstat 13 files changed, 49 insertions(+), 227 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,21 @@
 2009-12-16  Eric Blake  <ebb9@byu.net>
 
+	fcntl: use to simplify other modules
+	* modules/cloexec (Depends-on): Add fcntl.
+	* modules/fchdir (Depends-on): Likewise.
+	* modules/fd-safer-flag (Depends-on): Likewise.
+	* modules/unistd-safer (Depends-on): Likewise.
+	* modules/dup3 (configure.ac): Set module indicator.
+	* m4/fchdir.m4 (gl_FUNC_FCHDIR): Replace fcntl if fchdir is
+	missing.
+	* lib/fchdir.c (_gl_register_dup): Fix comment.
+	* lib/cloexec.c (dup_cloexec): Simplify, by relying on fcntl.
+	* lib/dup-safer.c (dup_safer): Likewise.
+	* lib/dup-safer-flag.c (dup_safer_flag): Likewise.
+	* lib/dup3.c (dup3): Likewise.
+	* tests/test-fchdir.c (main): Enhance test.
+	Fixes a dup_cloexec bug reported by Ondřej Vašík.
+
 	fcntl: port portions of fcntl to mingw
 	* m4/fcntl.m4 (gl_FUNC_FCNTL): Also build fcntl.c on mingw.
 	* lib/fcntl.c (fcntl) <F_DUPFD, F_DUPFD_CLOEXEC, F_GETFD>: Provide
--- a/lib/cloexec.c
+++ b/lib/cloexec.c
@@ -26,14 +26,6 @@
 #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.
@@ -47,7 +39,7 @@
 int
 set_cloexec_flag (int desc, bool value)
 {
-#if defined F_GETFD && defined F_SETFD
+#ifdef F_SETFD
 
   int flags = fcntl (desc, F_GETFD, 0);
 
@@ -62,7 +54,7 @@
 
   return -1;
 
-#else
+#else /* !F_SETFD */
 
   /* Use dup2 to reject invalid file descriptors; the cloexec flag
      will be unaffected.  */
@@ -77,7 +69,7 @@
 
   /* There is nothing we can do on this kind of platform.  Punt.  */
   return 0;
-#endif
+#endif /* !F_SETFD */
 }
 
 
@@ -88,77 +80,5 @@
 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;
-  int mode;
-
-  if (old_handle == INVALID_HANDLE_VALUE
-      || (mode = setmode (fd, O_BINARY)) == -1)
-    {
-      /* 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;
-    }
-  setmode (fd, mode);
-
-  if (!DuplicateHandle (curr_process,               /* SourceProcessHandle */
-                        old_handle,                 /* SourceHandle */
-                        curr_process,               /* TargetProcessHandle */
-                        (PHANDLE) &new_handle,      /* TargetHandle */
-                        (DWORD) 0,                  /* DesiredAccess */
-                        FALSE,                      /* InheritHandle */
-                        DUPLICATE_SAME_ACCESS))     /* Options */
-    {
-      /* TODO: Translate GetLastError () into errno.  */
-      errno = EMFILE;
-      return -1;
-    }
-
-  nfd = _open_osfhandle ((long) new_handle, mode | O_NOINHERIT);
-  if (nfd < 0)
-    {
-      CloseHandle (new_handle);
-      errno = EMFILE;
-      return -1;
-    }
-
-#  if REPLACE_FCHDIR
-  if (0 <= nfd)
-    nfd = _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)
-    nfd = _gl_register_dup (fd, nfd);
-#  endif
-
-# else /* !F_DUPFD_CLOEXEC */
-  nfd = dup (fd);
-  if (0 <= nfd && set_cloexec_flag (nfd, true) < 0)
-    {
-      int saved_errno = errno;
-      close (nfd);
-      nfd = -1;
-      errno = saved_errno;
-    }
-# endif /* !F_DUPFD_CLOEXEC */
-
-  return nfd;
-
-#endif /* !_WIN32 */
+  return fcntl (fd, F_DUPFD_CLOEXEC, 0);
 }
--- a/lib/dup-safer-flag.c
+++ b/lib/dup-safer-flag.c
@@ -39,16 +39,6 @@
 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);
+  return fcntl (fd, (flag & O_CLOEXEC) ? F_DUPFD_CLOEXEC : F_DUPFD,
+                STDERR_FILENO + 1);
 }
--- a/lib/dup-safer.c
+++ b/lib/dup-safer.c
@@ -31,11 +31,5 @@
 int
 dup_safer (int fd)
 {
-#if defined F_DUPFD && !REPLACE_FCHDIR
   return fcntl (fd, F_DUPFD, STDERR_FILENO + 1);
-#else
-  /* fd_safer calls us back, but eventually the recursion unwinds and
-     does the right thing.  */
-  return fd_safer (dup (fd));
-#endif
 }
--- a/lib/dup3.c
+++ b/lib/dup3.c
@@ -74,7 +74,7 @@
   }
 #endif
 
-  if (oldfd < 0 || newfd < 0 || newfd >= getdtablesize ())
+  if (newfd < 0 || newfd >= getdtablesize () || fcntl (oldfd, F_GETFD) == -1)
     {
       errno = EBADF;
       return -1;
@@ -95,130 +95,23 @@
       return -1;
     }
 
-#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
-/* Native Woe32 API.  */
-
   if (flags & O_CLOEXEC)
     {
-      /* Neither dup() nor dup2() can create a file descriptor with
-         O_CLOEXEC = O_NOINHERIT set.  We need to use the low-level function
-         _open_osfhandle for this.  Iterate until all file descriptors less
-         than newfd are filled up.  */
-      HANDLE curr_process = GetCurrentProcess ();
-      HANDLE old_handle = (HANDLE) _get_osfhandle (oldfd);
-      unsigned char fds_to_close[OPEN_MAX_MAX / CHAR_BIT];
-      unsigned int fds_to_close_bound = 0;
       int result;
-
-      if (old_handle == INVALID_HANDLE_VALUE)
-        {
-          /* oldfd is not open, or is an unassigned standard file
-             descriptor.  */
-          errno = EBADF;
-          return -1;
-        }
-
       close (newfd);
-
-      for (;;)
+      result = fcntl (oldfd, F_DUPFD_CLOEXEC, newfd);
+      if (newfd < result)
         {
-          HANDLE new_handle;
-          int duplicated_fd;
-          unsigned int index;
-
-          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 = EBADF; /* arbitrary */
-              result = -1;
-              break;
-            }
-          duplicated_fd = _open_osfhandle ((long) new_handle, flags);
-          if (duplicated_fd < 0)
-            {
-              CloseHandle (new_handle);
-              result = -1;
-              break;
-            }
-          if (duplicated_fd > newfd)
-            /* Shouldn't happen, since newfd is still closed.  */
-            abort ();
-          if (duplicated_fd == newfd)
-            {
-              result = newfd;
-              break;
-            }
-
-          /* Set the bit duplicated_fd in fds_to_close[].  */
-          index = (unsigned int) duplicated_fd / CHAR_BIT;
-          if (index >= fds_to_close_bound)
-            {
-              if (index >= sizeof (fds_to_close))
-                /* Need to increase OPEN_MAX_MAX.  */
-                abort ();
-              memset (fds_to_close + fds_to_close_bound, '\0',
-                      index + 1 - fds_to_close_bound);
-              fds_to_close_bound = index + 1;
-            }
-          fds_to_close[index] |= 1 << ((unsigned int) duplicated_fd % CHAR_BIT);
+          close (result);
+          errno = EIO;
+          result = -1;
         }
-
-      /* Close the previous fds that turned out to be too small.  */
-      {
-        int saved_errno = errno;
-        unsigned int duplicated_fd;
-
-        for (duplicated_fd = 0;
-             duplicated_fd < fds_to_close_bound * CHAR_BIT;
-             duplicated_fd++)
-          if ((fds_to_close[duplicated_fd / CHAR_BIT]
-               >> (duplicated_fd % CHAR_BIT))
-              & 1)
-            close (duplicated_fd);
-
-        errno = saved_errno;
-      }
-
-#if REPLACE_FCHDIR
-      if (result == newfd)
-        result = _gl_register_dup (oldfd, newfd);
-#endif
-      return result;
+      if (result < 0)
+        return -1;
     }
-
-  if (dup2 (oldfd, newfd) < 0)
+  else if (dup2 (oldfd, newfd) < 0)
     return -1;
 
-#else
-/* Unix API.  */
-
-  if (dup2 (oldfd, newfd) < 0)
-    return -1;
-
-  /* POSIX <http://www.opengroup.org/onlinepubs/9699919799/functions/dup.html>
-     says that initially, the FD_CLOEXEC flag is cleared on newfd.  */
-
-  if (flags & O_CLOEXEC)
-    {
-      int fcntl_flags;
-
-      if ((fcntl_flags = fcntl (newfd, F_GETFD, 0)) < 0
-          || fcntl (newfd, F_SETFD, fcntl_flags | FD_CLOEXEC) == -1)
-        {
-          int saved_errno = errno;
-          close (newfd);
-          errno = saved_errno;
-          return -1;
-        }
-    }
-
-#endif
-
 #if O_BINARY
   if (flags & O_BINARY)
     setmode (newfd, O_BINARY);
@@ -226,8 +119,5 @@
     setmode (newfd, O_TEXT);
 #endif
 
-#if REPLACE_FCHDIR
-  newfd = _gl_register_dup (oldfd, newfd);
-#endif
   return newfd;
 }
--- a/lib/fchdir.c
+++ b/lib/fchdir.c
@@ -168,10 +168,7 @@
    and fcntl.  Both arguments must be valid and distinct file
    descriptors.  Close NEWFD and return -1 if OLDFD is tracking a
    directory, but there is insufficient memory to track the same
-   directory in NEWFD; otherwise return NEWFD.
-
-   FIXME: Need to implement rpl_fcntl in gnulib, and have it call
-   this.  */
+   directory in NEWFD; otherwise return NEWFD.  */
 int
 _gl_register_dup (int oldfd, int newfd)
 {
--- a/m4/fchdir.m4
+++ b/m4/fchdir.m4
@@ -1,4 +1,4 @@
-# fchdir.m4 serial 12
+# fchdir.m4 serial 13
 dnl Copyright (C) 2006-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,
@@ -26,6 +26,7 @@
     gl_REPLACE_CLOSE
     gl_REPLACE_DUP2
     dnl dup3 is already unconditionally replaced
+    gl_REPLACE_FCNTL
     gl_REPLACE_DIRENT_H
     AC_CACHE_CHECK([whether open can visit directories],
       [gl_cv_func_open_directory_works],
--- a/modules/cloexec
+++ b/modules/cloexec
@@ -8,6 +8,7 @@
 
 Depends-on:
 dup2
+fcntl
 stdbool
 
 configure.ac:
--- a/modules/dup3
+++ b/modules/dup3
@@ -13,6 +13,7 @@
 
 configure.ac:
 gl_FUNC_DUP3
+gl_MODULE_INDICATOR([dup3])
 gl_UNISTD_MODULE_INDICATOR([dup3])
 
 Makefile.am:
--- a/modules/fchdir
+++ b/modules/fchdir
@@ -10,6 +10,7 @@
 dirent
 dirfd
 dup2
+fcntl
 fcntl-h
 include_next
 malloc-posix
--- a/modules/fd-safer-flag
+++ b/modules/fd-safer-flag
@@ -7,8 +7,9 @@
 lib/dup-safer-flag.c
 
 Depends-on:
+cloexec
+fcntl
 unistd-safer
-cloexec
 
 configure.ac:
 gl_MODULE_INDICATOR([fd-safer-flag])
--- a/modules/unistd-safer
+++ b/modules/unistd-safer
@@ -10,6 +10,7 @@
 m4/unistd-safer.m4
 
 Depends-on:
+fcntl
 unistd
 
 configure.ac:
--- a/tests/test-fchdir.c
+++ b/tests/test-fchdir.c
@@ -89,6 +89,15 @@
           ASSERT (dup_cloexec (fd) == new_fd);
           ASSERT (dup2 (new_fd, fd) == fd);
           ASSERT (close (new_fd) == 0);
+          ASSERT (fcntl (fd, F_DUPFD_CLOEXEC, new_fd) == new_fd);
+          ASSERT (close (fd) == 0);
+          ASSERT (fcntl (new_fd, F_DUPFD, fd) == fd);
+          ASSERT (close (new_fd) == 0);
+#if GNULIB_DUP3
+          ASSERT (dup3 (fd, new_fd, 0) == new_fd);
+          ASSERT (dup3 (new_fd, fd, 0) == fd);
+          ASSERT (close (new_fd) == 0);
+#endif
         }
     }