questions and maybe a patch about fdopendir in coreutils

[Paul Eggert]

A few things things about fdopendir.

1.  Yesterday, fdopendir was added to glibc.  The patch below attempts
to address this, but I have a few qualms about this since I haven't
tested it with glibc CVS, though -- I don't have that easily
available.

2.  I'm not sure what that AT_FDCWD test is doing in fdopendir's
implementation.  That behavior isn't documented in the Sun manual that
I can see.  (It's not in glibc either, but that's to be expected.)  As
far as I can see, coreutils never passes AT_FDCWD to fdopendir so
removing this test shouldn't change coreutils itself.

3.  The Sun documentation says fdopendir consumes its fd only if
fdopendir is successful.

4.  Is all this academic?  coreutils never uses lib/openat.c's
fdopendir that I can see.  getcwd.c uses fdopendir only if AT_FDCWD is
defined in the system headers, which means it doesn't use openat.c's
fdopendir, right?

Maybe fdopendir should simply be removed from openat.h and openat.c,
to avoid possible collisions with glibc?

But if not, here's a proposed patch:

2005-09-29  Paul Eggert  <address@hidden>

        * lib/openat.c (fdopendir): Do not define if HAVE_FDOPENDIR.
        Remove AT_FDCWD test.
        Do not consume the fd unless successful.
        * lib/openat.h (fdopendir): Do not define if HAVE_FDOPENDIR.
        * m4/openat.m4 (gl_FUNC_OPENAT): Check for fdopendir.

Index: lib/openat.c
===================================================================
RCS file: /fetish/cu/lib/openat.c,v
retrieving revision 1.13
diff -p -u -r1.13 openat.c
--- lib/openat.c.~1.13.~        2005-09-21 23:05:39.000000000 -0700
+++ lib/openat.c        2005-09-29 23:56:30.000000000 -0700
@@ -89,6 +89,8 @@ rpl_openat (int fd, char const *file, in
   return new_fd;
 }
 
+#if !HAVE_FDOPENDIR
+
 /* Replacement for Solaris' function by the same name.
    <http://www.google.com/search?q=fdopendir+site:docs.sun.com>
    Simulate it by doing save_cwd/fchdir/opendir(".")/restore_cwd.
@@ -100,7 +102,7 @@ rpl_openat (int fd, char const *file, in
    W A R N I N G:
    Unlike the other fd-related functions here, this one
    effectively consumes its FD parameter.  The caller should not
-   close or otherwise manipulate FD after calling this function.  */
+   close or otherwise manipulate FD if this function returns successfully.  */
 DIR *
 fdopendir (int fd)
 {
@@ -108,9 +110,6 @@ fdopendir (int fd)
   int saved_errno;
   DIR *dir;
 
-  if (fd == AT_FDCWD)
-    return opendir (".");
-
   if (save_cwd (&saved_cwd) != 0)
     openat_save_fail (errno);
 
@@ -118,7 +117,6 @@ fdopendir (int fd)
     {
       saved_errno = errno;
       free_cwd (&saved_cwd);
-      close (fd);
       errno = saved_errno;
       return NULL;
     }
@@ -130,12 +128,15 @@ fdopendir (int fd)
     openat_restore_fail (errno);
 
   free_cwd (&saved_cwd);
-  close (fd);
+  if (dir)
+    close (fd);
 
   errno = saved_errno;
   return dir;
 }
 
+#endif
+
 /* Replacement for Solaris' function by the same name.
    <http://www.google.com/search?q=fstatat+site:docs.sun.com>
    Simulate it by doing save_cwd/fchdir/(stat|lstat)/restore_cwd.
Index: lib/openat.h
===================================================================
RCS file: /fetish/cu/lib/openat.h,v
retrieving revision 1.11
diff -p -u -r1.11 openat.h
--- lib/openat.h        19 Sep 2005 15:44:57 -0000      1.11
+++ lib/openat.h        30 Sep 2005 06:47:03 -0000
@@ -46,7 +46,9 @@
 #  define __OPENAT_ID(y) __OPENAT_XCONCAT (__OPENAT_PREFIX, y)
 #  define openat __OPENAT_ID (openat)
 int openat (int fd, char const *file, int flags, /* mode_t mode */ ...);
-#  define fdopendir __OPENAT_ID (fdopendir)
+#  if ! HAVE_FDOPENDIR
+#   define fdopendir __OPENAT_ID (fdopendir)
+#  endif
 DIR *fdopendir (int fd);
 #  define fstatat __OPENAT_ID (fstatat)
 int fstatat (int fd, char const *file, struct stat *st, int flag);
Index: m4/openat.m4
===================================================================
RCS file: /fetish/cu/m4/openat.m4,v
retrieving revision 1.5
diff -p -u -r1.5 openat.m4
--- m4/openat.m4        14 Jun 2005 07:47:52 -0000      1.5
+++ m4/openat.m4        30 Sep 2005 06:47:03 -0000
@@ -1,4 +1,4 @@
-#serial 4
+#serial 5
 # See if we need to use our replacement for Solaris' openat function.
 
 dnl Copyright (C) 2004, 2005 Free Software Foundation, Inc.
@@ -13,6 +13,7 @@ AC_DEFUN([gl_FUNC_OPENAT],
   AC_LIBSOURCES([openat.c, openat.h, openat-die.c])
   AC_LIBOBJ([openat-die])
   AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS])
+  AC_CHECK_FUNCS_ONCE([fdopendir])
   AC_REPLACE_FUNCS(openat)
   case $ac_cv_func_openat in
   yes) ;;