fchdir fixes

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

fchdir is working on mingw, which is about the only modern portability
target that lacks fchdir, but it is sure easier to test on other platforms
than mingw.  So before making rpl_fcntl (and having it call
_gl_register_dup at the right places), I decided to try './configure
ac_cv_func_fchdir=no' on a system with fchdir.  That failed miserably, so
I'm pushing the following.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkseV8wACgkQ84KuGfSFAYD+BACcD6InQ581gnb6Amfly/vfZKwp
gsEAnRCqvH3KINSelKDmGK0Deo5dCTBq
=Qs3n
-----END PGP SIGNATURE-----

>From 21a0a9e3ef07b457b04d483f8dd216bbdf06e860 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 7 Dec 2009 21:08:17 -0700
Subject: [PATCH 1/2] dup2: fix logic bugs

If the platform has dup2, don't register with fchdir if the
destination was -1.

If the platform lacks dup2 (are there any these days?), then don't
close the destination unless the source is valid, make sure errno
is correct, and only register with fchdir on fcntl (since dup is
already overridden to do a registration).

* lib/dup2.c (dup2): Fix logic bugs.  Use HAVE_DUP2 rather than
REPLACE_DUP2 to decide when rpl_dup2 is needed.
* m4/dup2.m4 (gl_REPLACE_DUP2): Only define REPLACE_DUP2 when dup2
exists.
(gl_FUNC_DUP2): Drop unneeded AC_SUBST.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog  |    9 +++++++++
 lib/dup2.c |   30 ++++++++++++++++--------------
 m4/dup2.m4 |    8 ++++----
 3 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7b78dca..40d2dac 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2009-12-08  Eric Blake  <address@hidden>
+
+       dup2: fix logic bugs
+       * lib/dup2.c (dup2): Fix logic bugs.  Use HAVE_DUP2 rather than
+       REPLACE_DUP2 to decide when rpl_dup2 is needed.
+       * m4/dup2.m4 (gl_REPLACE_DUP2): Only define REPLACE_DUP2 when dup2
+       exists.
+       (gl_FUNC_DUP2): Drop unneeded AC_SUBST.
+
 2009-12-07  Eric Blake  <address@hidden>

        unlink: fix m4 detection
diff --git a/lib/dup2.c b/lib/dup2.c
index 140af1b..7ce49a2 100644
--- a/lib/dup2.c
+++ b/lib/dup2.c
@@ -32,7 +32,7 @@
 # include <windows.h>
 #endif

-#if REPLACE_DUP2
+#if HAVE_DUP2

 # undef dup2

@@ -70,14 +70,14 @@ rpl_dup2 (int fd, int desired_fd)
   /* Correct a cygwin 1.5.x errno value.  */
   else if (result == -1 && errno == EMFILE)
     errno = EBADF;
-#if REPLACE_FCHDIR
-  if (fd != desired_fd && result == desired_fd)
-    result = _gl_register_dup (fd, desired_fd);
-#endif
+# if REPLACE_FCHDIR
+  if (fd != desired_fd && result != -1)
+    result = _gl_register_dup (fd, result);
+# endif
   return result;
 }

-#else /* !REPLACE_DUP2 */
+#else /* !HAVE_DUP2 */

 /* On older platforms, dup2 did not exist.  */

@@ -102,19 +102,21 @@ dupfd (int fd, int desired_fd)
 int
 dup2 (int fd, int desired_fd)
 {
-  int result;
-  if (fd == desired_fd)
-    return fcntl (fd, F_GETFL) < 0 ? -1 : fd;
+  int result = fcntl (fd, F_GETFL) < 0 ? -1 : fd;
+  if (result == -1 || fd == desired_fd)
+    return result;
   close (desired_fd);
 # ifdef F_DUPFD
   result = fcntl (fd, F_DUPFD, desired_fd);
+#  if REPLACE_FCHDIR
+  if (0 <= result)
+    result = _gl_register_dup (fd, result);
+#  endif
 # else
   result = dupfd (fd, desired_fd);
 # endif
-#if REPLACE_FCHDIR
-  if (0 <= result)
-    result = _gl_register_dup (fd, desired_fd);
-#endif
+  if (result == -1 && (errno == EMFILE || errno == EINVAL))
+    errno = EBADF;
   return result;
 }
-#endif /* !REPLACE_DUP2 */
+#endif /* !HAVE_DUP2 */
diff --git a/m4/dup2.m4 b/m4/dup2.m4
index a74e915..26a6f60 100644
--- a/m4/dup2.m4
+++ b/m4/dup2.m4
@@ -1,4 +1,4 @@
-#serial 9
+#serial 10
 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,
@@ -46,13 +46,13 @@ AC_DEFUN([gl_FUNC_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
+  if test $ac_cv_func_dup2 = yes; then
+    REPLACE_DUP2=1
+  fi
   AC_LIBOBJ([dup2])
 ])
-- 
1.6.5.rc1


>From 2966d0d21fdc455b705e1b477dfe7e1b37f5f8e9 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 8 Dec 2009 06:13:05 -0700
Subject: [PATCH 2/2] fchdir: fix logic bugs

Configuring with ac_cv_func_fchdir=no on a system that has fchdir
and where open handles directories, just to test out the replacement
capabilities, uncovered an m4 test bug and a link failure on rpl_fstat.

* m4/fchdir.m4 (gl_FUNC_FCHDIR): Fix logic bug.
* tests/test-fchdir.c (main): Enhance test.
* lib/fchdir.c (rpl_fstat): Always provide if fchdir replacement
is in use.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog           |    6 ++++++
 lib/fchdir.c        |    7 +++----
 m4/fchdir.m4        |    4 ++--
 tests/test-fchdir.c |    8 +++++++-
 4 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 40d2dac..24d7e20 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2009-12-08  Eric Blake  <address@hidden>

+       fchdir: fix logic bugs
+       * m4/fchdir.m4 (gl_FUNC_FCHDIR): Fix logic bug.
+       * tests/test-fchdir.c (main): Enhance test.
+       * lib/fchdir.c (rpl_fstat): Always provide if fchdir replacement
+       is in use.
+
        dup2: fix logic bugs
        * lib/dup2.c (dup2): Fix logic bugs.  Use HAVE_DUP2 rather than
        REPLACE_DUP2 to decide when rpl_dup2 is needed.
diff --git a/lib/fchdir.c b/lib/fchdir.c
index 16b17b4..4cc0f33 100644
--- a/lib/fchdir.c
+++ b/lib/fchdir.c
@@ -216,16 +216,15 @@ _gl_directory_name (int fd)
 /* 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
-# undef fstat
+#undef fstat
 int
 rpl_fstat (int fd, struct stat *statbuf)
 {
-  if (0 <= fd && fd < dirs_allocated && dirs[fd].name != NULL)
+  if (REPLACE_OPEN_DIRECTORY
+      && 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().  */
diff --git a/m4/fchdir.m4 b/m4/fchdir.m4
index f0e4dc0..6243cc2 100644
--- a/m4/fchdir.m4
+++ b/m4/fchdir.m4
@@ -1,4 +1,4 @@
-# fchdir.m4 serial 10
+# fchdir.m4 serial 11
 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,
@@ -31,7 +31,7 @@ AC_DEFUN([gl_FUNC_FCHDIR],
     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);])],
+]], [return open(".", O_RDONLY) < 0;])],
         [gl_cv_func_open_directory_works=yes],
         [gl_cv_func_open_directory_works=no],
         [gl_cv_func_open_directory_works="guessing no"])])
diff --git a/tests/test-fchdir.c b/tests/test-fchdir.c
index 53d6631..0419f43 100644
--- a/tests/test-fchdir.c
+++ b/tests/test-fchdir.c
@@ -26,6 +26,8 @@
 #include <stdlib.h>
 #include <string.h>

+#include "cloexec.h"
+
 #define ASSERT(expr) \
   do                                                                        \
     {                                                                       \
@@ -82,7 +84,11 @@ main (void)
          int new_fd = dup (fd);
          ASSERT (0 <= new_fd);
          ASSERT (close (fd) == 0);
-         fd = new_fd;
+         ASSERT (dup2 (new_fd, fd) == fd);
+         ASSERT (close (new_fd) == 0);
+         ASSERT (dup_cloexec (fd) == new_fd);
+         ASSERT (dup2 (new_fd, fd) == fd);
+         ASSERT (close (new_fd) == 0);
        }
     }

-- 
1.6.5.rc1