From: Eric Blake Date: Wed, 16 Dec 2009 17:07:13 +0000 (-0700) Subject: fcntl: use to simplify other modules X-Git-Tag: v0.1~5030 X-Git-Url: http://erislabs.net/gitweb/?a=commitdiff_plain;h=e4ee142063a2f63f483d32b4ab87387db9130804;p=gnulib.git fcntl: use to simplify other modules Let fcntl do the work, instead of copying code into other modules. * modules/cloexec (Depends-on): Add fcntl. * modules/fchdir (Depends-on): Likewise. * modules/fd-safer-flag (Depends-on): Likewise. * modules/unistd-safer (Depends-on): Likewise. * modules/dup3 (configure.ac): Set module indicator. * m4/fchdir.m4 (gl_FUNC_FCHDIR): Replace fcntl if fchdir is missing. * lib/fchdir.c (_gl_register_dup): Fix comment. * lib/cloexec.c (dup_cloexec): Simplify, by relying on fcntl. * lib/dup-safer.c (dup_safer): Likewise. * lib/dup-safer-flag.c (dup_safer_flag): Likewise. * lib/dup3.c (dup3): Likewise. * tests/test-fchdir.c (main): Enhance test. Fixes a dup_cloexec bug reported by Ondřej Vašík. Signed-off-by: Eric Blake --- diff --git a/ChangeLog b/ChangeLog index 7117e5a19..329149933 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,21 @@ 2009-12-16 Eric Blake + fcntl: use to simplify other modules + * modules/cloexec (Depends-on): Add fcntl. + * modules/fchdir (Depends-on): Likewise. + * modules/fd-safer-flag (Depends-on): Likewise. + * modules/unistd-safer (Depends-on): Likewise. + * modules/dup3 (configure.ac): Set module indicator. + * m4/fchdir.m4 (gl_FUNC_FCHDIR): Replace fcntl if fchdir is + missing. + * lib/fchdir.c (_gl_register_dup): Fix comment. + * lib/cloexec.c (dup_cloexec): Simplify, by relying on fcntl. + * lib/dup-safer.c (dup_safer): Likewise. + * lib/dup-safer-flag.c (dup_safer_flag): Likewise. + * lib/dup3.c (dup3): Likewise. + * tests/test-fchdir.c (main): Enhance test. + Fixes a dup_cloexec bug reported by Ondřej Vašík. + fcntl: port portions of fcntl to mingw * m4/fcntl.m4 (gl_FUNC_FCNTL): Also build fcntl.c on mingw. * lib/fcntl.c (fcntl) : Provide diff --git a/lib/cloexec.c b/lib/cloexec.c index 69b45b4a3..ec3cd4e38 100644 --- a/lib/cloexec.c +++ b/lib/cloexec.c @@ -26,14 +26,6 @@ #include #include -#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ -/* Native Woe32 API. */ -# define WIN32_LEAN_AND_MEAN -# include -# include -#endif - - /* Set the `FD_CLOEXEC' flag of DESC if VALUE is true, or clear the flag if VALUE is false. Return 0 on success, or -1 on error with `errno' set. @@ -47,7 +39,7 @@ int set_cloexec_flag (int desc, bool value) { -#if defined F_GETFD && defined F_SETFD +#ifdef F_SETFD int flags = fcntl (desc, F_GETFD, 0); @@ -62,7 +54,7 @@ set_cloexec_flag (int desc, bool value) return -1; -#else +#else /* !F_SETFD */ /* Use dup2 to reject invalid file descriptors; the cloexec flag will be unaffected. */ @@ -77,7 +69,7 @@ set_cloexec_flag (int desc, bool value) /* There is nothing we can do on this kind of platform. Punt. */ return 0; -#endif +#endif /* !F_SETFD */ } @@ -88,77 +80,5 @@ set_cloexec_flag (int desc, bool value) int dup_cloexec (int fd) { - int nfd; - -#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ - - /* Native Woe32 API. */ - HANDLE curr_process = GetCurrentProcess (); - HANDLE old_handle = (HANDLE) _get_osfhandle (fd); - HANDLE new_handle; - int mode; - - if (old_handle == INVALID_HANDLE_VALUE - || (mode = setmode (fd, O_BINARY)) == -1) - { - /* fd is closed, or is open to no handle at all. - We cannot duplicate fd in this case, because _open_osfhandle - fails for an INVALID_HANDLE_VALUE argument. */ - errno = EBADF; - return -1; - } - setmode (fd, mode); - - if (!DuplicateHandle (curr_process, /* SourceProcessHandle */ - old_handle, /* SourceHandle */ - curr_process, /* TargetProcessHandle */ - (PHANDLE) &new_handle, /* TargetHandle */ - (DWORD) 0, /* DesiredAccess */ - FALSE, /* InheritHandle */ - DUPLICATE_SAME_ACCESS)) /* Options */ - { - /* TODO: Translate GetLastError () into errno. */ - errno = EMFILE; - return -1; - } - - nfd = _open_osfhandle ((long) new_handle, mode | O_NOINHERIT); - if (nfd < 0) - { - CloseHandle (new_handle); - errno = EMFILE; - return -1; - } - -# if REPLACE_FCHDIR - if (0 <= nfd) - nfd = _gl_register_dup (fd, nfd); -# endif - return nfd; - -#else /* !_WIN32 */ - - /* Unix API. */ - -# ifdef F_DUPFD_CLOEXEC - nfd = fcntl (fd, F_DUPFD_CLOEXEC, 0); -# if REPLACE_FCHDIR - if (0 <= nfd) - nfd = _gl_register_dup (fd, nfd); -# endif - -# else /* !F_DUPFD_CLOEXEC */ - nfd = dup (fd); - if (0 <= nfd && set_cloexec_flag (nfd, true) < 0) - { - int saved_errno = errno; - close (nfd); - nfd = -1; - errno = saved_errno; - } -# endif /* !F_DUPFD_CLOEXEC */ - - return nfd; - -#endif /* !_WIN32 */ + return fcntl (fd, F_DUPFD_CLOEXEC, 0); } diff --git a/lib/dup-safer-flag.c b/lib/dup-safer-flag.c index 3549d0d12..b76c6b63c 100644 --- a/lib/dup-safer-flag.c +++ b/lib/dup-safer-flag.c @@ -39,16 +39,6 @@ int dup_safer_flag (int fd, int flag) { - if (flag & O_CLOEXEC) - { -#if defined F_DUPFD_CLOEXEC && !REPLACE_FCHDIR - return fcntl (fd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1); -#else - /* fd_safer_flag calls us back, but eventually the recursion - unwinds and does the right thing. */ - fd = dup_cloexec (fd); - return fd_safer_flag (fd, flag); -#endif - } - return dup_safer (fd); + return fcntl (fd, (flag & O_CLOEXEC) ? F_DUPFD_CLOEXEC : F_DUPFD, + STDERR_FILENO + 1); } diff --git a/lib/dup-safer.c b/lib/dup-safer.c index bb11ba54a..ee29d3d69 100644 --- a/lib/dup-safer.c +++ b/lib/dup-safer.c @@ -31,11 +31,5 @@ int dup_safer (int fd) { -#if defined F_DUPFD && !REPLACE_FCHDIR return fcntl (fd, F_DUPFD, STDERR_FILENO + 1); -#else - /* fd_safer calls us back, but eventually the recursion unwinds and - does the right thing. */ - return fd_safer (dup (fd)); -#endif } diff --git a/lib/dup3.c b/lib/dup3.c index c61b9ba4b..85412c785 100644 --- a/lib/dup3.c +++ b/lib/dup3.c @@ -74,7 +74,7 @@ dup3 (int oldfd, int newfd, int flags) } #endif - if (oldfd < 0 || newfd < 0 || newfd >= getdtablesize ()) + if (newfd < 0 || newfd >= getdtablesize () || fcntl (oldfd, F_GETFD) == -1) { errno = EBADF; return -1; @@ -95,130 +95,23 @@ dup3 (int oldfd, int newfd, int flags) return -1; } -#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ -/* Native Woe32 API. */ - if (flags & O_CLOEXEC) { - /* Neither dup() nor dup2() can create a file descriptor with - O_CLOEXEC = O_NOINHERIT set. We need to use the low-level function - _open_osfhandle for this. Iterate until all file descriptors less - than newfd are filled up. */ - HANDLE curr_process = GetCurrentProcess (); - HANDLE old_handle = (HANDLE) _get_osfhandle (oldfd); - unsigned char fds_to_close[OPEN_MAX_MAX / CHAR_BIT]; - unsigned int fds_to_close_bound = 0; int result; - - if (old_handle == INVALID_HANDLE_VALUE) - { - /* oldfd is not open, or is an unassigned standard file - descriptor. */ - errno = EBADF; - return -1; - } - close (newfd); - - for (;;) + result = fcntl (oldfd, F_DUPFD_CLOEXEC, newfd); + if (newfd < result) { - HANDLE new_handle; - int duplicated_fd; - unsigned int index; - - if (!DuplicateHandle (curr_process, /* SourceProcessHandle */ - old_handle, /* SourceHandle */ - curr_process, /* TargetProcessHandle */ - (PHANDLE) &new_handle, /* TargetHandle */ - (DWORD) 0, /* DesiredAccess */ - FALSE, /* InheritHandle */ - DUPLICATE_SAME_ACCESS)) /* Options */ - { - errno = EBADF; /* arbitrary */ - result = -1; - break; - } - duplicated_fd = _open_osfhandle ((long) new_handle, flags); - if (duplicated_fd < 0) - { - CloseHandle (new_handle); - result = -1; - break; - } - if (duplicated_fd > newfd) - /* Shouldn't happen, since newfd is still closed. */ - abort (); - if (duplicated_fd == newfd) - { - result = newfd; - break; - } - - /* Set the bit duplicated_fd in fds_to_close[]. */ - index = (unsigned int) duplicated_fd / CHAR_BIT; - if (index >= fds_to_close_bound) - { - if (index >= sizeof (fds_to_close)) - /* Need to increase OPEN_MAX_MAX. */ - abort (); - memset (fds_to_close + fds_to_close_bound, '\0', - index + 1 - fds_to_close_bound); - fds_to_close_bound = index + 1; - } - fds_to_close[index] |= 1 << ((unsigned int) duplicated_fd % CHAR_BIT); + close (result); + errno = EIO; + result = -1; } - - /* Close the previous fds that turned out to be too small. */ - { - int saved_errno = errno; - unsigned int duplicated_fd; - - for (duplicated_fd = 0; - duplicated_fd < fds_to_close_bound * CHAR_BIT; - duplicated_fd++) - if ((fds_to_close[duplicated_fd / CHAR_BIT] - >> (duplicated_fd % CHAR_BIT)) - & 1) - close (duplicated_fd); - - errno = saved_errno; - } - -#if REPLACE_FCHDIR - if (result == newfd) - result = _gl_register_dup (oldfd, newfd); -#endif - return result; + if (result < 0) + return -1; } - - if (dup2 (oldfd, newfd) < 0) - return -1; - -#else -/* Unix API. */ - - if (dup2 (oldfd, newfd) < 0) + else if (dup2 (oldfd, newfd) < 0) return -1; - /* POSIX - says that initially, the FD_CLOEXEC flag is cleared on newfd. */ - - if (flags & O_CLOEXEC) - { - int fcntl_flags; - - if ((fcntl_flags = fcntl (newfd, F_GETFD, 0)) < 0 - || fcntl (newfd, F_SETFD, fcntl_flags | FD_CLOEXEC) == -1) - { - int saved_errno = errno; - close (newfd); - errno = saved_errno; - return -1; - } - } - -#endif - #if O_BINARY if (flags & O_BINARY) setmode (newfd, O_BINARY); @@ -226,8 +119,5 @@ dup3 (int oldfd, int newfd, int flags) setmode (newfd, O_TEXT); #endif -#if REPLACE_FCHDIR - newfd = _gl_register_dup (oldfd, newfd); -#endif return newfd; } diff --git a/lib/fchdir.c b/lib/fchdir.c index 5930940bd..8c0ff1362 100644 --- a/lib/fchdir.c +++ b/lib/fchdir.c @@ -168,10 +168,7 @@ _gl_register_fd (int fd, const char *filename) and fcntl. Both arguments must be valid and distinct file descriptors. Close NEWFD and return -1 if OLDFD is tracking a directory, but there is insufficient memory to track the same - directory in NEWFD; otherwise return NEWFD. - - FIXME: Need to implement rpl_fcntl in gnulib, and have it call - this. */ + directory in NEWFD; otherwise return NEWFD. */ int _gl_register_dup (int oldfd, int newfd) { diff --git a/m4/fchdir.m4 b/m4/fchdir.m4 index e0240a15f..fcdf62e92 100644 --- a/m4/fchdir.m4 +++ b/m4/fchdir.m4 @@ -1,4 +1,4 @@ -# fchdir.m4 serial 12 +# fchdir.m4 serial 13 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, @@ -26,6 +26,7 @@ AC_DEFUN([gl_FUNC_FCHDIR], gl_REPLACE_CLOSE gl_REPLACE_DUP2 dnl dup3 is already unconditionally replaced + gl_REPLACE_FCNTL gl_REPLACE_DIRENT_H AC_CACHE_CHECK([whether open can visit directories], [gl_cv_func_open_directory_works], diff --git a/modules/cloexec b/modules/cloexec index bb2767d88..88fb6d3cf 100644 --- a/modules/cloexec +++ b/modules/cloexec @@ -8,6 +8,7 @@ m4/cloexec.m4 Depends-on: dup2 +fcntl stdbool configure.ac: diff --git a/modules/dup3 b/modules/dup3 index 46650441d..07228a3bc 100644 --- a/modules/dup3 +++ b/modules/dup3 @@ -13,6 +13,7 @@ getdtablesize configure.ac: gl_FUNC_DUP3 +gl_MODULE_INDICATOR([dup3]) gl_UNISTD_MODULE_INDICATOR([dup3]) Makefile.am: diff --git a/modules/fchdir b/modules/fchdir index 5bae7d611..46b481fd1 100644 --- a/modules/fchdir +++ b/modules/fchdir @@ -10,6 +10,7 @@ close dirent dirfd dup2 +fcntl fcntl-h include_next malloc-posix diff --git a/modules/fd-safer-flag b/modules/fd-safer-flag index 887b79747..0a2df83ba 100644 --- a/modules/fd-safer-flag +++ b/modules/fd-safer-flag @@ -7,8 +7,9 @@ lib/fd-safer-flag.c lib/dup-safer-flag.c Depends-on: -unistd-safer cloexec +fcntl +unistd-safer configure.ac: gl_MODULE_INDICATOR([fd-safer-flag]) diff --git a/modules/unistd-safer b/modules/unistd-safer index 86e23abe0..dfa567215 100644 --- a/modules/unistd-safer +++ b/modules/unistd-safer @@ -10,6 +10,7 @@ lib/unistd-safer.h m4/unistd-safer.m4 Depends-on: +fcntl unistd configure.ac: diff --git a/tests/test-fchdir.c b/tests/test-fchdir.c index 19065591d..75819ebe6 100644 --- a/tests/test-fchdir.c +++ b/tests/test-fchdir.c @@ -89,6 +89,15 @@ main (void) ASSERT (dup_cloexec (fd) == new_fd); ASSERT (dup2 (new_fd, fd) == fd); ASSERT (close (new_fd) == 0); + ASSERT (fcntl (fd, F_DUPFD_CLOEXEC, new_fd) == new_fd); + ASSERT (close (fd) == 0); + ASSERT (fcntl (new_fd, F_DUPFD, fd) == fd); + ASSERT (close (new_fd) == 0); +#if GNULIB_DUP3 + ASSERT (dup3 (fd, new_fd, 0) == new_fd); + ASSERT (dup3 (new_fd, fd, 0) == fd); + ASSERT (close (new_fd) == 0); +#endif } }