bug-gnulib
[Top][All Lists]
Advanced

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

fchdir on mingw


From: Eric Blake
Subject: fchdir on mingw
Date: Mon, 31 Aug 2009 21:35:36 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

One of the main reasons the fchdir module was added was so that the openat 
module (and friends) would compile on mingw.  Well, it turns out that mingw has 
the (documented) limitation that open(dir,O_RDONLY) fails with EACCES, so 
rpl_open was never registering an fd visiting a directory in the first place, 
and the fchdir replacement (and thus openat and fts) failed to run on mingw.  
The first patch improves things by adding a fchdir unit test, and teaches 
rpl_open how to fake out opening a directory (the end result of 
using "/dev/null" under the hood means that read() on the fd gives the same EOF 
results as on Linux; and replacing fstat() means we can hide the fact that we 
used a dummy).

Techincally, I probably could have spent more time trying to figure out how to 
open a directory HANDLE using CreateFile with FILE_FLAG_BACKUP_SEMANTICS, but I 
couldn't quickly get that working.  I also wonder how _open_osfhandle would 
react to a directory HANDLE.  At any rate, since the method in the patch below 
passed the unit test, I'm not going to worry about the CreateFile approach.

The second patch fixes the fact that dup3 needs to play nicely with fchdir, and 
changes dup2 so that it is only overridden once, not twice, in gnulib sources.  
It also simplifies the logic in fchdir by not maintaining a saved_errno for 
every known fd, but instead aborting any attempt to open a directory up front 
with ENOMEM if we could not enlarge the map or strdup the directory name.  It 
is still possible to use fcntl(F_DUPFD{,_CLOEXEC}) to go behind fchdir's back 
at the moment, but until I finish an fcntl replacement module, that isn't too 
much to worry about.

I'm also working on a patch to split fdopendir into its own module (rather than 
being intimately tied to openat), as a prerequisite for implementing 
opendir_safer and "dirent--.h".  One improvement possible for fdopendir is that 
if fchdir is being emulated by gnulib, we can do a query into the fchdir 
metadata table to retrieve the directory name associated with an fd, rather 
than having to go through save_cwd/fchdir/opendir/restore_cwd (although the 
latter finally does work now on mingw, with just my first patch in place).

Also, it might still be possible to make dirfd work on mingw (right now, it 
always returns -1).  One way would be by adding a struct rpl_DIR which tracks 
an fd associated with a DIR, another way would be by adding a DIR* field to the 
fchdir metadata then doing a linear search for DIR pointer equality to find 
which fd is tied to a given DIR.  Since the maximum fd is not all that big, the 
O(n) cost of a reverse lookup is not too bad, and seems easier than an O(1) 
lookup by overriding struct DIR.  But with either approach, it would mean that 
fdopendir would need to change to keep the fd open as long as the DIR is open 
(rather than the current fdopendir policy of closing the user's fd up front), 
closedir would have to be responsible for closing the fd tied to a DIR, and we 
would have to decide whether it was better to always open an fd up front in 
opendir or just delay until the first dirfd (if any) for tying an fd to a DIR 
created by opendir.

So, here's the two patches I plan to apply soon...


From: Eric Blake <address@hidden>
Date: Mon, 31 Aug 2009 06:09:08 -0600
Subject: [PATCH 1/2] fchdir: port to mingw

* m4/fchdir.m4 (gl_FUNC_FCHDIR): Check for mingw bug.
* lib/open.c (open) [FCHDIR_REPLACEMENT]: If directories can't be
opened, then use a substitute.
* lib/sys_stat.in.h (fstat) [REPLACE_OPEN_DIRECTORY]: Declare
replacement.
* lib/fchdir.c (fstat) [REPLACE_OPEN_DIRECTORY]: Implement it.
(_gl_register_fd): No need to check stat if open already filters
all directories.
(fchdir): Fix error condition to match POSIX.
* modules/fchdir (Depends-on): Add sys_stat.
* doc/posix-functions/open.texi (open): Document the limitation.
* modules/fchdir-tests: New file.
* tests/test-fchdir.c: Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                     |   15 +++++++
 doc/posix-functions/open.texi |   11 ++++-
 lib/fchdir.c                  |   63 ++++++++++++++++++----------
 lib/open.c                    |   29 +++++++++++++-
 lib/sys_stat.in.h             |    4 ++
 m4/fchdir.m4                  |   13 +++++-
 modules/fchdir                |    1 +
 modules/fchdir-tests          |   11 +++++
 tests/test-fchdir.c           |   91 +++++++++++++++++++++++++++++++++++++++++
 9 files changed, 211 insertions(+), 27 deletions(-)
 create mode 100644 modules/fchdir-tests
 create mode 100644 tests/test-fchdir.c

diff --git a/ChangeLog b/ChangeLog
index 6e39cef..749215d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,20 @@
 2009-08-31  Eric Blake  <address@hidden>

+       fchdir: port to mingw
+       * m4/fchdir.m4 (gl_FUNC_FCHDIR): Check for mingw bug.
+       * lib/open.c (open) [FCHDIR_REPLACEMENT]: If directories can't be
+       opened, then use a substitute.
+       * lib/sys_stat.in.h (fstat) [REPLACE_OPEN_DIRECTORY]: Declare
+       replacement.
+       * lib/fchdir.c (fstat) [REPLACE_OPEN_DIRECTORY]: Implement it.
+       (_gl_register_fd): No need to check stat if open already filters
+       all directories.
+       (fchdir): Fix error condition to match POSIX.
+       * modules/fchdir (Depends-on): Add sys_stat.
+       * doc/posix-functions/open.texi (open): Document the limitation.
+       * modules/fchdir-tests: New file.
+       * tests/test-fchdir.c: Likewise.
+
        canonicalize: allow cross-testing from cygwin to mingw
        * modules/canonicalize-tests (configure.ac): Define HAVE_SYMLINK.
        (Makefile.am): Pass it through TESTS_ENVIRONMENT.
diff --git a/doc/posix-functions/open.texi b/doc/posix-functions/open.texi
index 27990a0..6a919d8 100644
--- a/doc/posix-functions/open.texi
+++ b/doc/posix-functions/open.texi
@@ -4,9 +4,9 @@ open

 POSIX specification: @url
{http://www.opengroup.org/onlinepubs/9699919799/functions/open.html}

-Gnulib module: open
+Gnulib module: open, fchdir

-Portability problems fixed by Gnulib:
+Portability problems fixed by the Gnulib module open:
 @itemize
 @item
 This function does not fail when the file name argument ends in a slash
@@ -18,6 +18,13 @@ open
 recognize the @file{/dev/null} filename.
 @end itemize

+Portability problems fixed by the Gnulib module fchdir:
address@hidden
address@hidden
+On Windows platforms (excluding Cygwin), this function fails to open a
+read-only descriptor for directories.
address@hidden itemize
+
 Portability problems not fixed by Gnulib:
 @itemize
 @item
diff --git a/lib/fchdir.c b/lib/fchdir.c
index d51d963..cedbcde 100644
--- a/lib/fchdir.c
+++ b/lib/fchdir.c
@@ -30,8 +30,17 @@

 #include "canonicalize.h"

+#ifndef REPLACE_OPEN_DIRECTORY
+# define REPLACE_OPEN_DIRECTORY 0
+#endif
+
 /* This replacement assumes that a directory is not renamed while opened
-   through a file descriptor.  */
+   through a file descriptor.
+
+   FIXME: On mingw, this would be possible to enforce if we were to
+   also open a HANDLE to each directory currently visited by a file
+   descriptor, since mingw refuses to rename any in-use file system
+   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.  
*/
@@ -77,6 +86,8 @@ ensure_dirs_slot (size_t fd)
 /* Hook into the gnulib replacements for open() and close() to keep track
    of the open file descriptors.  */

+/* Close FD, cleaning up any fd to name mapping if fd was visiting a
+   directory.  */
 void
 _gl_unregister_fd (int fd)
 {
@@ -89,6 +100,9 @@ _gl_unregister_fd (int fd)
     }
 }

+/* 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
 _gl_register_fd (int fd, const char *filename)
 {
@@ -96,14 +110,28 @@ _gl_register_fd (int fd, const char *filename)

   ensure_dirs_slot (fd);
   if (fd < dirs_allocated
-      && fstat (fd, &statbuf) >= 0 && S_ISDIR (statbuf.st_mode))
+      && (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;
+        dirs[fd].saved_errno = errno;
     }
 }

+/* Return stat information about FD in STATBUF.  Needed when
+   rpl_open() used a dummy file to work around an open() that can't
+   normally visit directories.  */
+#if REPLACE_OPEN_DIRECTORY
+int
+rpl_fstat (int fd, struct stat *statbuf)
+{
+  if (0 <= fd && fd <= dirs_allocated && dirs[fd].name != NULL)
+    return stat (dirs[fd].name, statbuf);
+  return fstat (fd, statbuf);
+}
+#endif
+
 /* Override opendir() and closedir(), to keep track of the open file
    descriptors.  Needed because there is a function dirfd().  */

@@ -218,27 +246,16 @@ rpl_dup2_fchdir (int oldfd, int newfd)
 int
 fchdir (int fd)
 {
-  if (fd >= 0)
+  if (fd >= 0 && fd < dirs_allocated && dirs[fd].name != NULL)
+    return chdir (dirs[fd].name);
+  /* At this point, fd is either invalid, or open but not a directory.
+     If dup2 fails, errno is correctly EBADF.  */
+  if (0 <= fd)
     {
-      if (fd < dirs_allocated)
-       {
-         if (dirs[fd].name != NULL)
-           return chdir (dirs[fd].name);
-         else
-           {
-             errno = dirs[fd].saved_errno;
-             return -1;
-           }
-       }
-      else
-       {
-         errno = ENOMEM;
-         return -1;
-       }
+      if (dup2 (fd, fd) == fd)
+        errno = ENOTDIR;
     }
   else
-    {
-      errno = EBADF;
-      return -1;
-    }
+    errno = EBADF;
+  return -1;
 }
diff --git a/lib/open.c b/lib/open.c
index 326e6d1..5cfef1f 100644
--- a/lib/open.c
+++ b/lib/open.c
@@ -39,6 +39,10 @@ orig_open (const char *filename, int flags, mode_t mode)
 #include <sys/types.h>
 #include <sys/stat.h>

+#ifndef REPLACE_OPEN_DIRECTORY
+# define REPLACE_OPEN_DIRECTORY 0
+#endif
+
 int
 open (const char *filename, int flags, ...)
 {
@@ -98,6 +102,29 @@ open (const char *filename, int flags, ...)

   fd = orig_open (filename, flags, mode);

+#ifdef FCHDIR_REPLACEMENT
+  /* Implementing fchdir and fdopendir requires the ability to open a
+     directory file descriptor.  If open doesn't support that (as on
+     mingw), we use a dummy file that behaves the same as directories
+     on Linux (ie. always reports EOF on attempts to read()), and
+     override fstat() in fchdir.c to hide the fact that we have a
+     dummy.  */
+  if (REPLACE_OPEN_DIRECTORY && fd < 0 && errno == EACCES
+      && (mode & O_ACCMODE) == O_RDONLY)
+    {
+      struct stat statbuf;
+      if (stat (filename, &statbuf) == 0 && S_ISDIR (statbuf.st_mode))
+        {
+          /* Maximum recursion depth of 1.  */
+          fd = open ("/dev/null", flags, mode);
+          if (0 <= fd)
+            _gl_register_fd (fd, filename);
+        }
+      else
+        errno = EACCES;
+    }
+#endif
+
 #if OPEN_TRAILING_SLASH_BUG
   /* If the filename ends in a slash and fd does not refer to a directory,
      then fail.
@@ -129,7 +156,7 @@ open (const char *filename, int flags, ...)
 #endif

 #ifdef FCHDIR_REPLACEMENT
-  if (fd >= 0)
+  if (!REPLACE_OPEN_DIRECTORY && 0 <= fd)
     _gl_register_fd (fd, filename);
 #endif

diff --git a/lib/sys_stat.in.h b/lib/sys_stat.in.h
index da965d0..9d511da 100644
--- a/lib/sys_stat.in.h
+++ b/lib/sys_stat.in.h
@@ -302,6 +302,10 @@ extern int rpl_lstat (const char *name, struct stat *buf);
    lstat (p, b))
 #endif

+#if defined FCHDIR_REPLACEMENT && REPLACE_OPEN_DIRECTORY
+# define fstat rpl_fstat
+extern int fstat (int fd, struct stat *buf);
+#endif

 #if @REPLACE_MKDIR@
 # undef mkdir
diff --git a/m4/fchdir.m4 b/m4/fchdir.m4
index d5dd3e2..49e6634 100644
--- a/m4/fchdir.m4
+++ b/m4/fchdir.m4
@@ -1,4 +1,4 @@
-# fchdir.m4 serial 7
+# fchdir.m4 serial 8
 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,
@@ -18,6 +18,17 @@ AC_DEFUN([gl_FUNC_FCHDIR],
     gl_REPLACE_OPEN
     gl_REPLACE_CLOSE
     gl_REPLACE_DIRENT_H
+    AC_CACHE_CHECK([whether open can visit directories],
+      [gl_cv_func_open_directory_works],
+      [AC_RUN_IFELSE([AC_LANG_PROGRAM([[#include <fcntl.h>
+]], [return open(".", O_RDONLY);])],
+        [gl_cv_func_open_directory_works=yes],
+        [gl_cv_func_open_directory_works=no],
+        [gl_cv_func_open_directory_works="guessing no"])])
+    if test "$gl_cv_func_open_directory_works" != yes; then
+      AC_DEFINE([REPLACE_OPEN_DIRECTORY], [1], [Define to 1 if open() should
+work around the inability to open a directory.])
+    fi
   fi
 ])

diff --git a/modules/fchdir b/modules/fchdir
index 17e842c..d3fe0e5 100644
--- a/modules/fchdir
+++ b/modules/fchdir
@@ -15,6 +15,7 @@ fcntl-h
 include_next
 open
 strdup
+sys_stat
 unistd

 configure.ac:
diff --git a/modules/fchdir-tests b/modules/fchdir-tests
new file mode 100644
index 0000000..f3bb8b2
--- /dev/null
+++ b/modules/fchdir-tests
@@ -0,0 +1,11 @@
+Files:
+tests/test-fchdir.c
+
+Depends-on:
+getcwd
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-fchdir
+check_PROGRAMS += test-fchdir
diff --git a/tests/test-fchdir.c b/tests/test-fchdir.c
new file mode 100644
index 0000000..b361d0d
--- /dev/null
+++ b/tests/test-fchdir.c
@@ -0,0 +1,91 @@
+/* Test changing to a directory named by a file descriptor.
+   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 <unistd.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#define ASSERT(expr) \
+  do                                                                        \
+    {                                                                       \
+      if (!(expr))                                                          \
+        {                                                                   \
+          fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
+          fflush (stderr);                                                  \
+          abort ();                                                         \
+        }                                                                   \
+    }                                                                       \
+  while (0)
+
+int
+main ()
+{
+  char *cwd = getcwd (NULL, 0);
+  int fd = open (".", O_RDONLY);
+  int i;
+
+  ASSERT (cwd);
+  ASSERT (0 <= fd);
+
+  /* Check for failure cases.  */
+  {
+    int bad_fd = open ("/dev/null", O_RDONLY);
+    ASSERT (0 <= bad_fd);
+    errno = 0;
+    ASSERT (fchdir (bad_fd) == -1);
+    ASSERT (errno == ENOTDIR);
+    ASSERT (close (bad_fd) == 0);
+    errno = 0;
+    ASSERT (fchdir (-1) == -1);
+    ASSERT (errno == EBADF);
+  }
+
+  /* Repeat test twice, once in '.' and once in '..'.  */
+  for (i = 0; i < 2; i++)
+    {
+      ASSERT (chdir (".." + 1 - i) == 0);
+      ASSERT (fchdir (fd) == 0);
+      {
+       size_t len = strlen (cwd) + 1;
+       char *new_dir = malloc (len);
+       ASSERT (new_dir);
+       ASSERT (getcwd (new_dir, len) == new_dir);
+       ASSERT (strcmp (cwd, new_dir) == 0);
+       free (new_dir);
+      }
+
+      /* For second iteration, use a cloned fd, to ensure that dup
+        remembers whether an fd was associated with a directory.  */
+      if (!i)
+       {
+         int new_fd = dup (fd);
+         ASSERT (0 <= new_fd);
+         ASSERT (close (fd) == 0);
+         fd = new_fd;
+       }
+    }
+
+  free (cwd);
+  return 0;
+}
-- 
1.6.3.2


>From cf22836a549bd29e2eac3f38de4b26cce7c79dee Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 31 Aug 2009 14:20:03 -0600
Subject: [PATCH 2/2] 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 <address@hidden>
---
 ChangeLog       |   19 ++++++
 lib/dup2.c      |   16 ++++-
 lib/dup3.c      |   11 ++++
 lib/fchdir.c    |  179 ++++++++++++++++++++++++-------------------------------
 lib/fcntl.in.h  |    2 +-
 lib/open.c      |    4 +-
 lib/unistd.in.h |    8 +--
 m4/dup2.m4      |   12 +++-
 m4/fchdir.m4    |    4 +-
 modules/fchdir  |    5 +-
 10 files changed, 142 insertions(+), 118 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 749215d..0d2bfd3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,24 @@
 2009-08-31  Eric Blake  <address@hidden>

+       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.
+
        fchdir: port to mingw
        * m4/fchdir.m4 (gl_FUNC_FCHDIR): Check for mingw bug.
        * lib/open.c (open) [FCHDIR_REPLACEMENT]: If directories can't be
diff --git a/lib/dup2.c b/lib/dup2.c
index fb3ffb0..a513e5b 100644
--- a/lib/dup2.c
+++ b/lib/dup2.c
@@ -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 */
diff --git a/lib/dup3.c b/lib/dup3.c
index f730e81..1f5b1b8 100644
--- a/lib/dup3.c
+++ b/lib/dup3.c
@@ -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;
 }
diff --git a/lib/fchdir.c b/lib/fchdir.c
index cedbcde..523d0a3 100644
--- 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,18 +44,18 @@
    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)
@@ -68,19 +69,16 @@ ensure_dirs_slot (size_t 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
@@ -93,30 +91,68 @@ _gl_unregister_fd (int fd)
 {
   if (fd >= 0 && fd < dirs_allocated)
     {
-      if (dirs[fd].name != NULL)
-       free (dirs[fd].name);
+      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
@@ -157,13 +193,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)
@@ -171,75 +212,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().  */

diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h
index 8b92521..959be44 100644
--- a/lib/fcntl.in.h
+++ b/lib/fcntl.in.h
@@ -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
diff --git a/lib/open.c b/lib/open.c
index 5cfef1f..02dd12d 100644
--- a/lib/open.c
+++ b/lib/open.c
@@ -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;
diff --git a/lib/unistd.in.h b/lib/unistd.in.h
index e81d35c..f0b5cc4 100644
--- a/lib/unistd.in.h
+++ b/lib/unistd.in.h
@@ -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


diff --git a/m4/dup2.m4 b/m4/dup2.m4
index 2e846c0..816a734 100644
--- 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 @@ 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])
+])
diff --git a/m4/fchdir.m4 b/m4/fchdir.m4
index 49e6634..bcaf056 100644
--- 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 @@ 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],
diff --git a/modules/fchdir b/modules/fchdir
index d3fe0e5..8a1cd1c 100644
--- a/modules/fchdir
+++ b/modules/fchdir
@@ -13,8 +13,11 @@ dirfd
 dup2
 fcntl-h
 include_next
+malloc-posix
 open
-strdup
+realloc-posix
+stdbool
+strdup-posix
 sys_stat
 unistd

-- 
1.6.3.2







reply via email to

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