-----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 [email protected] -----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 <[email protected]> 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 <[email protected]> --- 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 <[email protected]> + + 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> --- 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 <[email protected]> + 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
