fdopendir incompatibility with POSIX, with proposed patch

[Paul Eggert]

While coding up recent changes to GNU tar, I discovered a
POSIX-emulation problem in gnulib's implementation of fdopendir.
I had written code like this:


       DIR *dp = fdopendir (fd);
       int fd1 = openat (fd, "subdir", O_RDONLY);

This works with POSIX fdopendir, but the gnulib replacement for
fdopendir might close fd.  This is listed under a "W A R N I N G" in
fdopendir.c, but I didn't see the warning (I didn't even know about
gnulib fdopendir) so my 'tar' implementation was buggy.  The bug was
pretty hard to debug (it occurred only on old-fashioned hosts with
bad debuggers ...).

How about the following patch to help ameliorate this problem?  It
would have fixed the bug for GNU tar, anyway.  (I have reworked GNU
tar to use a different approach, so this patch is not needed for GNU
tar, but still it should help avoid future gotchas like this.)  This
patch won't work in general (multithreaded apps, signal handers) but
it should work for single-threaded apps whose signal handers don't open
or close files.

2010-09-12  Paul Eggert  <address@hidden>

        fdopendir: arrange for result to have the desired file descriptor
        * lib/fdopendir.c (fdopendir): Turn this into a wrapper around
        its guts, most of which got moved into fdopendir_internal.
        Arrange for result to have the asked-for file descriptor,
        assuming no race conditions.
        (fdopendir_internal): New static function, containing the previous
        code of fdopendir, except that it does not close FD.

--- old/lib/fdopendir.c 2010-09-11 23:19:52.718275000 -0700
+++ new/lib/fdopendir.c 2010-09-12 15:04:50.101385000 -0700
@@ -33,6 +33,8 @@
 #  include "dirent--.h"
 # endif
 
+static DIR *fdopendir_internal (int);
+
 /* Replacement for Solaris' function by the same name.
    <http://www.google.com/search?q=fdopendir+site:docs.sun.com>
    First, try to simulate it via opendir ("/proc/self/fd/FD").  Failing
@@ -40,18 +42,48 @@
    save_cwd/fchdir/opendir(".")/restore_cwd.
    If either the save_cwd or the restore_cwd fails (relatively unlikely),
    then give a diagnostic and exit nonzero.
+
+   If successful, the resulting stream is based on FD in
+   implementations where streams are based on file descriptors and in
+   applications where no other thread or signal handler allocates or
+   frees file descriptors.  In other cases, consult dirfd on the result
+   to find out whether FD is still being used.
+
    Otherwise, this function works just like Solaris' fdopendir.
 
    W A R N I N G:
-   Unlike other fd-related functions, this one effectively consumes
-   its FD parameter.  The caller should not close or otherwise
-   manipulate FD if this function returns successfully.  Also, this
-   implementation does not guarantee that dirfd(fdopendir(n))==n;
-   the open directory stream may use a clone of FD, or have no
-   associated fd at all.  */
+
+   Unlike other fd-related functions, this one places constraints on FD.
+   If this function returns successfully, FD is under control of the
+   dirent.h system, and the caller should not close or modify the state of
+   FD other than by the dirent.h functions.  */
 DIR *
 fdopendir (int fd)
 {
+  int dupfd = dup (fd);
+  if (dupfd < 0)
+    return NULL;
+  else
+    {
+      /* Make sure that FD is closed and all file descriptors less
+        than FD are open, and then call fdopendir_internal on a dup
+        of FD.  That way, barring race conditions, fdopendir_internal
+        will return a stream whose file descriptor is FD.  */
+      DIR *dir = (dupfd < fd
+                 ? fdopendir (fd)
+                 : (close (fd), fdopendir_internal (dupfd)));
+      int saved_errno = errno;
+      close (dupfd);
+      errno = saved_errno;
+      return dir;
+    }
+}
+
+/* Like fdopendir, except the result controls a clone of FD.
+   It is the caller's responsibility to close both FD and the result.  */
+static DIR *
+fdopendir_internal (int fd)
+{
   int saved_errno;
   DIR *dir;
 
@@ -100,8 +132,6 @@ fdopendir (int fd)
 # endif /* !REPLACE_FCHDIR */
     }
 
-  if (dir)
-    close (fd);
   if (proc_file != buf)
     free (proc_file);
   errno = saved_errno;