fchdir: simplify error handling, and support dup3
authorEric Blake <ebb9@byu.net>
Mon, 31 Aug 2009 20:20:03 +0000 (14:20 -0600)
committerEric Blake <ebb9@byu.net>
Thu, 3 Sep 2009 01:14:43 +0000 (19:14 -0600)
* modules/fchdir (Depends-on): Use strdup-posix, not strdup.  Add
stdbool, malloc-posix, realloc-posix.
* lib/fchdir.c (struct dir_info_t): Delete saved_errno.
(ensure_dirs_slot): Return false on allocation failure.
(rpl_dup2): Delete.
(_gl_register_dup): New function.
(_gl_unregister_fd, rpl_opendir, rpl_dup): Update callers.
(_gl_register_fd): Close fd on allocation failure.
* lib/fcntl.in.h (_gl_register_fd): Update signature.
* lib/unistd.in.h (_gl_register_dup) [FCHDIR_REPLACEMENT]: New
prototype.
(rpl_dup2_fchdir): Delete prototype.
* lib/open.c (open): Update caller.
* lib/dup2.c (dup2): Track fchdir metadata.
* lib/dup3.c (dup3): Likewise.
* m4/dup2.m4 (gl_REPLACE_DUP2): New macro.
* m4/fchdir.m4 (gl_FUNC_FCHDIR): Use it.

Signed-off-by: Eric Blake <ebb9@byu.net>
ChangeLog
lib/dup2.c
lib/dup3.c
lib/fchdir.c
lib/fcntl.in.h
lib/open.c
lib/unistd.in.h
m4/dup2.m4
m4/fchdir.m4
modules/fchdir

index 23f193e..6a45887 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2009-09-02  Eric Blake  <ebb9@byu.net>
+
+       fchdir: simplify error handling, and support dup3
+       * modules/fchdir (Depends-on): Use strdup-posix, not strdup.  Add
+       stdbool, malloc-posix, realloc-posix.
+       * lib/fchdir.c (struct dir_info_t): Delete saved_errno.
+       (ensure_dirs_slot): Return false on allocation failure.
+       (rpl_dup2): Delete.
+       (_gl_register_dup): New function.
+       (_gl_unregister_fd, rpl_opendir, rpl_dup): Update callers.
+       (_gl_register_fd): Close fd on allocation failure.
+       * lib/fcntl.in.h (_gl_register_fd): Update signature.
+       * lib/unistd.in.h (_gl_register_dup) [FCHDIR_REPLACEMENT]: New
+       prototype.
+       (rpl_dup2_fchdir): Delete prototype.
+       * lib/open.c (open): Update caller.
+       * lib/dup2.c (dup2): Track fchdir metadata.
+       * lib/dup3.c (dup3): Likewise.
+       * m4/dup2.m4 (gl_REPLACE_DUP2): New macro.
+       * m4/fchdir.m4 (gl_FUNC_FCHDIR): Use it.
+
 2009-09-02  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>
 
        * gnulib-tool (func_create_testdir, func_create_megatestdir): Use
index fb3ffb0..a513e5b 100644 (file)
@@ -70,6 +70,10 @@ rpl_dup2 (int fd, int desired_fd)
   /* Correct a cygwin 1.5.x errno value.  */
   else if (result == -1 && errno == EMFILE)
     errno = EBADF;
+#ifdef FCHDIR_REPLACEMENT
+  if (fd != desired_fd && result == desired_fd)
+    result = _gl_register_dup (fd, desired_fd);
+#endif
   return result;
 }
 
@@ -98,13 +102,19 @@ dupfd (int fd, int desired_fd)
 int
 dup2 (int fd, int desired_fd)
 {
+  int result;
   if (fd == desired_fd)
-    return fd;
+    return fcntl (fd, F_GETFL) < 0 ? -1 : fd;
   close (desired_fd);
 # ifdef F_DUPFD
-  return fcntl (fd, F_DUPFD, desired_fd);
+  result = fcntl (fd, F_DUPFD, desired_fd);
 # else
-  return dupfd (fd, desired_fd);
+  result = dupfd (fd, desired_fd);
 # endif
+#ifdef FCHDIR_REPLACEMENT
+  if (0 <= result)
+    result = _gl_register_dup (fd, desired_fd);
+#endif
+  return result;
 }
 #endif /* !REPLACE_DUP2 */
index f730e81..3d6f940 100644 (file)
@@ -63,6 +63,10 @@ dup3 (int oldfd, int newfd, int flags)
        if (!(result < 0 && errno == ENOSYS))
          {
            have_dup3_really = 1;
+#ifdef FCHDIR_REPLACEMENT
+           if (0 <= result)
+             result = _gl_register_dup (oldfd, newfd);
+#endif
            return result;
          }
        have_dup3_really = -1;
@@ -180,6 +184,10 @@ dup3 (int oldfd, int newfd, int flags)
        errno = saved_errno;
       }
 
+#ifdef FCHDIR_REPLACEMENT
+      if (result == newfd)
+       result = _gl_register_dup (oldfd, newfd);
+#endif
       return result;
     }
 
@@ -218,5 +226,8 @@ dup3 (int oldfd, int newfd, int flags)
     setmode (newfd, O_TEXT);
 #endif
 
+#ifdef FCHDIR_REPLACEMENT
+  newfd = _gl_register_dup (oldfd, newfd);
+#endif
   return newfd;
 }
index 170505f..9b64e02 100644 (file)
 /* Specification.  */
 #include <unistd.h>
 
+#include <assert.h>
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
-#include <stdarg.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/types.h>
    object.  */
 
 /* Array of file descriptors opened.  If it points to a directory, it stores
-   info about this directory; otherwise it stores an errno value of ENOTDIR.  */
+   info about this directory.  */
 typedef struct
 {
   char *name;       /* Absolute name of the directory, or NULL.  */
-  int saved_errno;  /* If name == NULL: The error code describing the failure
-                      reason.  */
+  /* FIXME - add a DIR* member to make dirfd possible on mingw?  */
 } dir_info_t;
 static dir_info_t *dirs;
 static size_t dirs_allocated;
 
-/* Try to ensure dirs has enough room for a slot at index fd.  */
-static void
+/* Try to ensure dirs has enough room for a slot at index fd.  Return
+   false and set errno to ENOMEM on allocation failure.  */
+static bool
 ensure_dirs_slot (size_t fd)
 {
   if (fd >= dirs_allocated)
     {
       size_t new_allocated;
       dir_info_t *new_dirs;
-      size_t i;
 
       new_allocated = 2 * dirs_allocated + 1;
       if (new_allocated <= fd)
        new_allocated = fd + 1;
       new_dirs =
        (dirs != NULL
-        ? (dir_info_t *) realloc (dirs, new_allocated * sizeof (dir_info_t))
-        : (dir_info_t *) malloc (new_allocated * sizeof (dir_info_t)));
-      if (new_dirs != NULL)
-       {
-         for (i = dirs_allocated; i < new_allocated; i++)
-           {
-             new_dirs[i].name = NULL;
-             new_dirs[i].saved_errno = ENOTDIR;
-           }
-         dirs = new_dirs;
-         dirs_allocated = new_allocated;
-       }
+        ? (dir_info_t *) realloc (dirs, new_allocated * sizeof *dirs)
+        : (dir_info_t *) malloc (new_allocated * sizeof *dirs));
+      if (new_dirs == NULL)
+        return false;
+      memset (new_dirs + dirs_allocated, 0,
+              (new_allocated - dirs_allocated) * sizeof *dirs);
+      dirs = new_dirs;
+      dirs_allocated = new_allocated;
     }
+  return true;
 }
 
 /* Hook into the gnulib replacements for open() and close() to keep track
@@ -95,27 +92,66 @@ _gl_unregister_fd (int fd)
     {
       free (dirs[fd].name);
       dirs[fd].name = NULL;
-      dirs[fd].saved_errno = ENOTDIR;
     }
 }
 
-/* Mark FD as visiting FILENAME.  FD must be positive, and refer to an
-   open file descriptor.  If REPLACE_OPEN_DIRECTORY is non-zero, this
-   should only be called if FD is visiting a directory.  */
-void
+/* Mark FD as visiting FILENAME.  FD must be non-negative, and refer
+   to an open file descriptor.  If REPLACE_OPEN_DIRECTORY is non-zero,
+   this should only be called if FD is visiting a directory.  Close FD
+   and return -1 if there is insufficient memory to track the
+   directory name; otherwise return FD.  */
+int
 _gl_register_fd (int fd, const char *filename)
 {
   struct stat statbuf;
 
-  ensure_dirs_slot (fd);
-  if (fd < dirs_allocated
-      && (REPLACE_OPEN_DIRECTORY
-          || (fstat (fd, &statbuf) >= 0 && S_ISDIR (statbuf.st_mode))))
+  assert (0 <= fd);
+  if (REPLACE_OPEN_DIRECTORY
+      || (fstat (fd, &statbuf) == 0 && S_ISDIR (statbuf.st_mode)))
+    {
+      if (!ensure_dirs_slot (fd)
+          || (dirs[fd].name = canonicalize_file_name (filename)) == NULL)
+        {
+          int saved_errno = errno;
+          close (fd);
+          errno = saved_errno;
+          return -1;
+        }
+    }
+  return fd;
+}
+
+/* Mark NEWFD as a duplicate of OLDFD; useful from dup, dup2, dup3,
+   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.  */
+int
+_gl_register_dup (int oldfd, int newfd)
+{
+  assert (0 <= oldfd && 0 <= newfd && oldfd != newfd);
+  if (oldfd < dirs_allocated && dirs[oldfd].name)
     {
-      dirs[fd].name = canonicalize_file_name (filename);
-      if (dirs[fd].name == NULL)
-        dirs[fd].saved_errno = errno;
+      /* Duplicated a directory; must ensure newfd is allocated.  */
+      if (!ensure_dirs_slot (newfd)
+          || (dirs[newfd].name = strdup (dirs[oldfd].name)) == NULL)
+        {
+          int saved_errno = errno;
+          close (newfd);
+          errno = saved_errno;
+          newfd = -1;
+        }
     }
+  else if (newfd < dirs_allocated)
+    {
+      /* Duplicated a non-directory; ensure newfd is cleared.  */
+      free (dirs[newfd].name);
+      dirs[newfd].name = NULL;
+    }
+  return newfd;
 }
 
 /* Return stat information about FD in STATBUF.  Needed when
@@ -156,13 +192,18 @@ rpl_opendir (const char *filename)
   if (dp != NULL)
     {
       int fd = dirfd (dp);
-      if (fd >= 0)
-       _gl_register_fd (fd, filename);
+      if (0 <= fd && _gl_register_fd (fd, filename) != fd)
+        {
+          int saved_errno = errno;
+          closedir (dp);
+          errno = saved_errno;
+          return NULL;
+        }
     }
   return dp;
 }
 
-/* Override dup() and dup2(), to keep track of open file descriptors.  */
+/* Override dup(), to keep track of open file descriptors.  */
 
 int
 rpl_dup (int oldfd)
@@ -170,75 +211,11 @@ rpl_dup (int oldfd)
 {
   int newfd = dup (oldfd);
 
-  if (oldfd >= 0 && newfd >= 0)
-    {
-      ensure_dirs_slot (newfd);
-      if (newfd < dirs_allocated)
-       {
-         if (oldfd < dirs_allocated)
-           {
-             if (dirs[oldfd].name != NULL)
-               {
-                 dirs[newfd].name = strdup (dirs[oldfd].name);
-                 if (dirs[newfd].name == NULL)
-                   dirs[newfd].saved_errno = ENOMEM;
-               }
-             else
-               {
-                 dirs[newfd].name = NULL;
-                 dirs[newfd].saved_errno = dirs[oldfd].saved_errno;
-               }
-           }
-         else
-           {
-             dirs[newfd].name = NULL;
-             dirs[newfd].saved_errno = ENOMEM;
-           }
-       }
-    }
+  if (0 <= newfd)
+    newfd = _gl_register_dup (oldfd, newfd);
   return newfd;
 }
 
-/* Our <unistd.h> replacement overrides dup2 twice; be sure to pick
-   the one we want.  */
-#if REPLACE_DUP2
-# undef dup2
-# define dup2 rpl_dup2
-#endif
-
-int
-rpl_dup2_fchdir (int oldfd, int newfd)
-{
-  int retval = dup2 (oldfd, newfd);
-
-  if (retval >= 0 && newfd != oldfd)
-    {
-      ensure_dirs_slot (newfd);
-      if (newfd < dirs_allocated)
-       {
-         if (oldfd < dirs_allocated)
-           {
-             if (dirs[oldfd].name != NULL)
-               {
-                 dirs[newfd].name = strdup (dirs[oldfd].name);
-                 if (dirs[newfd].name == NULL)
-                   dirs[newfd].saved_errno = ENOMEM;
-               }
-             else
-               {
-                 dirs[newfd].name = NULL;
-                 dirs[newfd].saved_errno = dirs[oldfd].saved_errno;
-               }
-           }
-         else
-           {
-             dirs[newfd].name = NULL;
-             dirs[newfd].saved_errno = ENOMEM;
-           }
-       }
-    }
-  return retval;
-}
 
 /* Implement fchdir() in terms of chdir().  */
 
index 8b92521..959be44 100644 (file)
@@ -60,7 +60,7 @@ extern int open (const char *filename, int flags, ...);
 
 #ifdef FCHDIR_REPLACEMENT
 /* gnulib internal function.  */
-extern void _gl_register_fd (int fd, const char *filename);
+extern int _gl_register_fd (int fd, const char *filename);
 #endif
 
 #ifdef __cplusplus
index 5cfef1f..02dd12d 100644 (file)
@@ -118,7 +118,7 @@ open (const char *filename, int flags, ...)
           /* Maximum recursion depth of 1.  */
           fd = open ("/dev/null", flags, mode);
           if (0 <= fd)
-            _gl_register_fd (fd, filename);
+            fd = _gl_register_fd (fd, filename);
         }
       else
         errno = EACCES;
@@ -157,7 +157,7 @@ open (const char *filename, int flags, ...)
 
 #ifdef FCHDIR_REPLACEMENT
   if (!REPLACE_OPEN_DIRECTORY && 0 <= fd)
-    _gl_register_fd (fd, filename);
+    fd = _gl_register_fd (fd, filename);
 #endif
 
   return fd;
index e81d35c..f0b5cc4 100644 (file)
@@ -248,12 +248,6 @@ extern int fchdir (int /*fd*/);
 #  define dup rpl_dup
 extern int dup (int);
 
-#  if @REPLACE_DUP2@
-#   undef dup2
-#  endif
-#  define dup2 rpl_dup2_fchdir
-extern int dup2 (int, int);
-
 # endif
 #elif defined GNULIB_POSIXCHECK
 # undef fchdir
@@ -624,6 +618,8 @@ extern ssize_t write (int fd, const void *buf, size_t count);
 #ifdef FCHDIR_REPLACEMENT
 /* gnulib internal function.  */
 extern void _gl_unregister_fd (int fd);
+/* gnulib internal function.  */
+extern int _gl_register_dup (int oldfd, int newfd);
 #endif
 
 
index 2e846c0..816a734 100644 (file)
@@ -1,4 +1,4 @@
-#serial 7
+#serial 8
 dnl Copyright (C) 2002, 2005, 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,
@@ -37,10 +37,16 @@ AC_DEFUN([gl_FUNC_DUP2],
         esac])
       ])
     if test "$gl_cv_func_dup2_works" = no; then
-      REPLACE_DUP2=1
-      AC_LIBOBJ([dup2])
+      gl_REPLACE_DUP2
     fi
   fi
   AC_DEFINE_UNQUOTED([REPLACE_DUP2], [$REPLACE_DUP2],
     [Define to 1 if dup2 returns 0 instead of the target fd.])
 ])
+
+AC_DEFUN([gl_REPLACE_DUP2],
+[
+  AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
+  REPLACE_DUP2=1
+  AC_LIBOBJ([dup2])
+])
index 49e6634..bcaf056 100644 (file)
@@ -1,4 +1,4 @@
-# fchdir.m4 serial 8
+# fchdir.m4 serial 9
 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,
@@ -17,6 +17,8 @@ AC_DEFUN([gl_FUNC_FCHDIR],
       [Define if gnulib's fchdir() replacement is used.])
     gl_REPLACE_OPEN
     gl_REPLACE_CLOSE
+    gl_REPLACE_DUP2
+    dnl dup3 is already unconditionally replaced
     gl_REPLACE_DIRENT_H
     AC_CACHE_CHECK([whether open can visit directories],
       [gl_cv_func_open_directory_works],
index d3fe0e5..8a1cd1c 100644 (file)
@@ -13,8 +13,11 @@ dirfd
 dup2
 fcntl-h
 include_next
+malloc-posix
 open
-strdup
+realloc-posix
+stdbool
+strdup-posix
 sys_stat
 unistd