changeset 11932:4073da4a848c

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. Signed-off-by: Eric Blake <ebb9@byu.net>
author Eric Blake <ebb9@byu.net>
date Mon, 31 Aug 2009 14:20:03 -0600
parents d42b3b6f11d3
children 1ffad224c413
files 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
diffstat 10 files changed, 143 insertions(+), 117 deletions(-) [+]
line wrap: on
line diff
--- 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
--- a/lib/dup2.c
+++ b/lib/dup2.c
@@ -70,6 +70,10 @@
   /* 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 @@
 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 */
--- a/lib/dup3.c
+++ b/lib/dup3.c
@@ -63,6 +63,10 @@
 	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 @@
 	errno = saved_errno;
       }
 
+#ifdef FCHDIR_REPLACEMENT
+      if (result == newfd)
+	result = _gl_register_dup (oldfd, newfd);
+#endif
       return result;
     }
 
@@ -218,5 +226,8 @@
     setmode (newfd, O_TEXT);
 #endif
 
+#ifdef FCHDIR_REPLACEMENT
+  newfd = _gl_register_dup (oldfd, newfd);
+#endif
   return newfd;
 }
--- a/lib/fchdir.c
+++ b/lib/fchdir.c
@@ -19,10 +19,11 @@
 /* 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>
@@ -43,44 +44,40 @@
    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 @@
     {
       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)))
     {
-      dirs[fd].name = canonicalize_file_name (filename);
-      if (dirs[fd].name == NULL)
-        dirs[fd].saved_errno = errno;
+      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)
+    {
+      /* 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 @@
   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 @@
 {
   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().  */
 
--- a/lib/fcntl.in.h
+++ b/lib/fcntl.in.h
@@ -60,7 +60,7 @@
 
 #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
--- a/lib/open.c
+++ b/lib/open.c
@@ -118,7 +118,7 @@
           /* 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 @@
 
 #ifdef FCHDIR_REPLACEMENT
   if (!REPLACE_OPEN_DIRECTORY && 0 <= fd)
-    _gl_register_fd (fd, filename);
+    fd = _gl_register_fd (fd, filename);
 #endif
 
   return fd;
--- a/lib/unistd.in.h
+++ b/lib/unistd.in.h
@@ -248,12 +248,6 @@
 #  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 @@
 #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
 
 
--- a/m4/dup2.m4
+++ b/m4/dup2.m4
@@ -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 @@
 	 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])
+])
--- a/m4/fchdir.m4
+++ b/m4/fchdir.m4
@@ -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 @@
       [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],
--- a/modules/fchdir
+++ b/modules/fchdir
@@ -13,8 +13,11 @@
 dup2
 fcntl-h
 include_next
+malloc-posix
 open
-strdup
+realloc-posix
+stdbool
+strdup-posix
 sys_stat
 unistd