bug-gnulib
[Top][All Lists]
Advanced

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

Re: fdopendir failure on MacOS X with GNU tar 1.25


From: tsteven4
Subject: Re: fdopendir failure on MacOS X with GNU tar 1.25
Date: Mon, 08 Nov 2010 06:38:12 -0700
User-agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.12) Gecko/20101027 Thunderbird/3.1.6

Paul,

The revised fdopendir.c you sent resolved the tar-1.25 test issue with test 37:
37: scarce file descriptors                         ok
The issue with test 35 remains, but I believe you have indicated that is another problem.
35: no need to save dir with unreadable . and .. FAILED (extrac09.at:25)
No new failures resulted from the change to fdopendir.c:
## ------------- ##
## Test results. ##
## ------------- ##

ERROR: 102 tests were run,
1 failed unexpectedly.
16 tests were skipped.
Thanks for the work on the test failure.

Please let me know if you need any additional information, or patch testing, or even pre-release testing on Mac OS X 10.6.

Steve


On 11/7/10 11:12 PM, Paul Eggert wrote:
(This is following up on recent bug reports sent to bug-tar:

<http://lists.gnu.org/archive/html/bug-tar/2010-10/msg00084.html>
<http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00000.html>
<http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00038.html>.

but it appears to be a gnulib bug so I'm CC'ing this to bug-gnulib.)

On 11/07/2010 09:49 AM, tsteven4 wrote:

I attached the output of "sudo dtruss -n tar -f -a>  &
tar125_dtruss.txt" while running "make check TESTSUITEFLAGS=37".
Thanks for sending that.  In looking through the dtruss output I see a
bug in gnulib's fdopendir.c implementation: when fdopendir(N) is
invoked, and N is the maximum openable file descriptor, and there are
some unopened file descriptors less than N, then fdopendir first uses
'dup' to consume these unopened file descriptors, and then invokes
save_cwd to save the working directory.  But save_cwd consumes a file
descriptor, so it fails with errno == EMFILE.  The fix is to invoke
save_cwd before invoking 'dup'.

Could you please try the following patch, which implements this fix?
I am enclosing a copy of the new fdopendir.c; perhaps it'd be easiest
if you simply copied this new version over tar-1.25/gnu/fdopendir.c,
and then rebuilt 'tar'.

Thanks.

> From 83fb034e8c087a7278c0d3a5686d9c384afa34b8 Mon Sep 17 00:00:00 2001
From: Paul Eggert<address@hidden>
Date: Sun, 7 Nov 2010 21:58:54 -0800
Subject: [PATCH] fdopendir: fix bug on MacOS X when low on file descriptors

* lib/fdopendir.c (REPLACE_FCHDIR): #define to 0 if not defined.
(fdopendir_with_dup, fd_clone_opendir): Now have extra CWD arg.
All callers changed.
(fdopendir): Invoke save_cwd at the top level, not after using
multiple dup() calls to use up file descriptors.  Then retry
fdopendir_with_dup.  This avoids failure with EMFILE if FD is 1
less than the maximum number of open file descriptors, because
save_cwd fails with errno == EMFILE.  Problem reported by tsteven4
on Mac OS X 10.6.4 for tar 1.24
<http://lists.gnu.org/archive/html/bug-tar/2010-10/msg00084.html>
<http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00000.html>
and for tar 1.25
<http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00038.html>.
---
  ChangeLog       |   18 ++++++++
  lib/fdopendir.c |  124 ++++++++++++++++++++++++++----------------------------
  2 files changed, 78 insertions(+), 64 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4eb90a3..4d9c4ea 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2010-11-07  Paul Eggert<address@hidden>
+
+       fdopendir: fix bug on MacOS X when low on file descriptors
+
+       * lib/fdopendir.c (REPLACE_FCHDIR): #define to 0 if not defined.
+       (fdopendir_with_dup, fd_clone_opendir): Now have extra CWD arg.
+       All callers changed.
+       (fdopendir): Invoke save_cwd at the top level, not after using
+       multiple dup() calls to use up file descriptors.  Then retry
+       fdopendir_with_dup.  This avoids failure with EMFILE if FD is 1
+       less than the maximum number of open file descriptors, because
+       save_cwd fails with errno == EMFILE.  Problem reported by tsteven4
+       on Mac OS X 10.6.4 for tar 1.24
+       <http://lists.gnu.org/archive/html/bug-tar/2010-10/msg00084.html>
+       <http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00000.html>
+       and for tar 1.25
+       <http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00038.html>.
+
  2010-11-07  Bruno Haible<address@hidden>

        vasnprintf: Support I flag on glibc systems.
diff --git a/lib/fdopendir.c b/lib/fdopendir.c
index 4d2935f..e565087 100644
--- a/lib/fdopendir.c
+++ b/lib/fdopendir.c
@@ -33,12 +33,16 @@
  #  include "dirent--.h"
  # endif

-static DIR *fdopendir_with_dup (int, int);
-static DIR *fd_clone_opendir (int);
+# ifndef REPLACE_FCHDIR
+#  define REPLACE_FCHDIR 0
+# endif
+
+static DIR *fdopendir_with_dup (int, int, struct saved_cwd const *);
+static DIR *fd_clone_opendir (int, struct saved_cwd const *);

  /* Replacement for POSIX fdopendir.

-   First, try to simulate it via opendir ("/proc/self/fd/FD").  Failing
+   First, try to simulate it via opendir ("/proc/self/fd/...").  Failing
     that, simulate it by using fchdir metadata, or by doing
     save_cwd/fchdir/opendir(".")/restore_cwd.
     If either the save_cwd or the restore_cwd fails (relatively unlikely),
@@ -61,7 +65,24 @@ static DIR *fd_clone_opendir (int);
  DIR *
  fdopendir (int fd)
  {
-  return fdopendir_with_dup (fd, -1);
+  DIR *dir = fdopendir_with_dup (fd, -1, NULL);
+
+  if (! REPLACE_FCHDIR&&  ! dir)
+    {
+      int saved_errno = errno;
+      if (EXPECTED_ERRNO (saved_errno))
+        {
+          struct saved_cwd cwd;
+          if (save_cwd (&cwd) != 0)
+            openat_save_fail (errno);
+          dir = fdopendir_with_dup (fd, -1,&cwd);
+          saved_errno = errno;
+          free_cwd (&cwd);
+          errno = saved_errno;
+        }
+    }
+
+  return dir;
  }

  /* Like fdopendir, except that if OLDER_DUPFD is not -1, it is known
@@ -70,9 +91,13 @@ fdopendir (int fd)
     function makes sure that FD is closed and all file descriptors less
     than FD are open, and then calls fd_clone_opendir on a dup of FD.
     That way, barring race conditions, fd_clone_opendir returns a
-   stream whose file descriptor is FD.  */
+   stream whose file descriptor is FD.
+
+   If REPLACE_CHDIR or CWD is null, use opendir ("/proc/self/fd/...",
+   falling back on fchdir metadata.  Otherwise, CWD is a saved version
+   of the working directory; use fchdir/opendir(".")/restore_cwd(CWD).  */
  static DIR *
-fdopendir_with_dup (int fd, int older_dupfd)
+fdopendir_with_dup (int fd, int older_dupfd, struct saved_cwd const *cwd)
  {
    int dupfd = dup (fd);
    if (dupfd<  0&&  errno == EMFILE)
@@ -85,13 +110,13 @@ fdopendir_with_dup (int fd, int older_dupfd)
        int saved_errno;
        if (dupfd<  fd - 1&&  dupfd != older_dupfd)
          {
-          dir = fdopendir_with_dup (fd, dupfd);
+          dir = fdopendir_with_dup (fd, dupfd, cwd);
            saved_errno = errno;
          }
        else
          {
            close (fd);
-          dir = fd_clone_opendir (dupfd);
+          dir = fd_clone_opendir (dupfd, cwd);
            saved_errno = errno;
            if (! dir)
              {
@@ -112,74 +137,45 @@ fdopendir_with_dup (int fd, int older_dupfd)
     the caller's responsibility both to close FD and (if the result is
     not null) to closedir the result.  */
  static DIR *
-fd_clone_opendir (int fd)
+fd_clone_opendir (int fd, struct saved_cwd const *cwd)
  {
-  int saved_errno;
-  DIR *dir;
-
-  char buf[OPENAT_BUFFER_SIZE];
-  char *proc_file = openat_proc_name (buf, fd, ".");
-  if (proc_file)
-    {
-      dir = opendir (proc_file);
-      saved_errno = errno;
-    }
-  else
+  if (REPLACE_FCHDIR || ! cwd)
      {
-      dir = NULL;
-      saved_errno = EOPNOTSUPP;
-    }
-
-  /* If the syscall fails with an expected errno value, resort to
-     save_cwd/restore_cwd.  */
-  if (! dir&&  EXPECTED_ERRNO (saved_errno))
-    {
-# if REPLACE_FCHDIR
-      const char *name = _gl_directory_name (fd);
-      if (name)
-        dir = opendir (name);
-      saved_errno = errno;
-# else /* !REPLACE_FCHDIR */
-
-      /* Occupy the destination FD slot, so that save_cwd cannot hijack it.  */
-      struct saved_cwd saved_cwd;
-      int fd_reserve = dup (fd);
-      if (fd_reserve<  0)
+      DIR *dir = NULL;
+      int saved_errno = EOPNOTSUPP;
+      char buf[OPENAT_BUFFER_SIZE];
+      char *proc_file = openat_proc_name (buf, fd, ".");
+      if (proc_file)
          {
+          dir = opendir (proc_file);
            saved_errno = errno;
-          dir = NULL;
-          goto fail;
+          if (proc_file != buf)
+            free (proc_file);
          }
-
-      if (save_cwd (&saved_cwd) != 0)
-        openat_save_fail (errno);
-
-      /* Liberate the target file descriptor, so that opendir uses it.  */
-      close (fd_reserve);
-
-      if (fchdir (fd) != 0)
+# if REPLACE_FCHDIR
+      if (! dir&&  EXPECTED_ERRNO (saved_errno))
          {
-          dir = NULL;
-          saved_errno = errno;
+          char const *name = _gl_directory_name (fd);
+          return (name ? opendir (name) : NULL);
          }
+# endif
+      errno = saved_errno;
+      return dir;
+    }
+  else
+    {
+      if (fchdir (fd) != 0)
+        return NULL;
        else
          {
-          dir = opendir (".");
-          saved_errno = errno;
-
-          if (restore_cwd (&saved_cwd) != 0)
+          DIR *dir = opendir (".");
+          int saved_errno = errno;
+          if (restore_cwd (cwd) != 0)
              openat_restore_fail (errno);
+          errno = saved_errno;
+          return dir;
          }
-
-      free_cwd (&saved_cwd);
-# endif /* !REPLACE_FCHDIR */
      }
-
- fail:
-  if (proc_file != buf)
-    free (proc_file);
-  errno = saved_errno;
-  return dir;
  }

  #else /* HAVE_FDOPENDIR */



reply via email to

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