Eric Blake <ebb9 <at> byu.net> writes: > +/* 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 > +int > +rpl_fstat (int fd, struct stat *statbuf) > +{ > + if (0 <= fd && fd <= dirs_allocated && dirs[fd].name != NULL) > + return stat (dirs[fd].name, statbuf);
Ouch. Off-by-one error, which can lead to segfault on mingw when fd == dirs_allocated. Here's the correction, along with the rest of my rebased pending patches. Any comments before I apply? Stay tuned for dirent-safer built on top of this. Eric Blake (5): fchdir: fix off-by-one bug in previous patch fchdir: simplify error handling, and support dup3 fchdir: use more consistent macro convention fdopendir: split into its own module fdopendir: optimize on mingw >From 2fc6c30f4fdda7b373a16dd849cfebcc4cc7669f Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Tue, 1 Sep 2009 10:06:44 -0600 Subject: [PATCH 1/5] fchdir: fix off-by-one bug in previous patch * lib/fchdir.c (rpl_fstat): Use correct bounds. (_gl_unregister_fd): Delete useless if. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 6 ++++++ lib/fchdir.c | 5 ++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index d53ed5d..7b67cf4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2009-09-01 Eric Blake <e...@byu.net> + + fchdir: fix off-by-one bug in previous patch + * lib/fchdir.c (rpl_fstat): Use correct bounds. + (_gl_unregister_fd): Delete useless if. + 2009-09-01 Simon Josefsson <si...@josefsson.org> * modules/arpa_inet: Use $(MKDIR_P) instead of @mkdi...@. diff --git a/lib/fchdir.c b/lib/fchdir.c index cedbcde..170505f 100644 --- a/lib/fchdir.c +++ b/lib/fchdir.c @@ -93,8 +93,7 @@ _gl_unregister_fd (int fd) { if (fd >= 0 && fd < dirs_allocated) { - if (dirs[fd].name != NULL) - free (dirs[fd].name); + free (dirs[fd].name); dirs[fd].name = NULL; dirs[fd].saved_errno = ENOTDIR; } @@ -126,7 +125,7 @@ _gl_register_fd (int fd, const char *filename) int rpl_fstat (int fd, struct stat *statbuf) { - if (0 <= fd && fd <= dirs_allocated && dirs[fd].name != NULL) + if (0 <= fd && fd < dirs_allocated && dirs[fd].name != NULL) return stat (dirs[fd].name, statbuf); return fstat (fd, statbuf); } -- 1.6.3.2 >From 21fc064f72d5acb6c326022b4b77818703b1c897 Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Mon, 31 Aug 2009 14:20:03 -0600 Subject: [PATCH 2/5] fchdir: simplify error handling, and support dup3 * modules/fchdir (Depends-on): Use strdup-posix, not strdup. Add stdbool, malloc-posix, realloc-posix. * lib/fchdir.c (struct dir_info_t): Delete saved_errno. (ensure_dirs_slot): Return false on allocation failure. (rpl_dup2): Delete. (_gl_register_dup): New function. (_gl_unregister_fd, rpl_opendir, rpl_dup): Update callers. (_gl_register_fd): Close fd on allocation failure. * lib/fcntl.in.h (_gl_register_fd): Update signature. * lib/unistd.in.h (_gl_register_dup) [FCHDIR_REPLACEMENT]: New prototype. (rpl_dup2_fchdir): Delete prototype. * lib/open.c (open): Update caller. * lib/dup2.c (dup2): Track fchdir metadata. * lib/dup3.c (dup3): Likewise. * m4/dup2.m4 (gl_REPLACE_DUP2): New macro. * m4/fchdir.m4 (gl_FUNC_FCHDIR): Use it. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 19 ++++++ lib/dup2.c | 16 ++++- lib/dup3.c | 11 ++++ lib/fchdir.c | 177 ++++++++++++++++++++++++------------------------------- lib/fcntl.in.h | 2 +- lib/open.c | 4 +- lib/unistd.in.h | 8 +-- m4/dup2.m4 | 12 +++- m4/fchdir.m4 | 4 +- modules/fchdir | 5 +- 10 files changed, 141 insertions(+), 117 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7b67cf4..6a0b18e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,24 @@ 2009-09-01 Eric Blake <e...@byu.net> + fchdir: simplify error handling, and support dup3 + * modules/fchdir (Depends-on): Use strdup-posix, not strdup. Add + stdbool, malloc-posix, realloc-posix. + * lib/fchdir.c (struct dir_info_t): Delete saved_errno. + (ensure_dirs_slot): Return false on allocation failure. + (rpl_dup2): Delete. + (_gl_register_dup): New function. + (_gl_unregister_fd, rpl_opendir, rpl_dup): Update callers. + (_gl_register_fd): Close fd on allocation failure. + * lib/fcntl.in.h (_gl_register_fd): Update signature. + * lib/unistd.in.h (_gl_register_dup) [FCHDIR_REPLACEMENT]: New + prototype. + (rpl_dup2_fchdir): Delete prototype. + * lib/open.c (open): Update caller. + * lib/dup2.c (dup2): Track fchdir metadata. + * lib/dup3.c (dup3): Likewise. + * m4/dup2.m4 (gl_REPLACE_DUP2): New macro. + * m4/fchdir.m4 (gl_FUNC_FCHDIR): Use it. + fchdir: fix off-by-one bug in previous patch * lib/fchdir.c (rpl_fstat): Use correct bounds. (_gl_unregister_fd): Delete useless if. diff --git a/lib/dup2.c b/lib/dup2.c index fb3ffb0..a513e5b 100644 --- a/lib/dup2.c +++ b/lib/dup2.c @@ -70,6 +70,10 @@ rpl_dup2 (int fd, int desired_fd) /* Correct a cygwin 1.5.x errno value. */ else if (result == -1 && errno == EMFILE) errno = EBADF; +#ifdef FCHDIR_REPLACEMENT + if (fd != desired_fd && result == desired_fd) + result = _gl_register_dup (fd, desired_fd); +#endif return result; } @@ -98,13 +102,19 @@ dupfd (int fd, int desired_fd) int dup2 (int fd, int desired_fd) { + int result; if (fd == desired_fd) - return fd; + return fcntl (fd, F_GETFL) < 0 ? -1 : fd; close (desired_fd); # ifdef F_DUPFD - return fcntl (fd, F_DUPFD, desired_fd); + result = fcntl (fd, F_DUPFD, desired_fd); # else - return dupfd (fd, desired_fd); + result = dupfd (fd, desired_fd); # endif +#ifdef FCHDIR_REPLACEMENT + if (0 <= result) + result = _gl_register_dup (fd, desired_fd); +#endif + return result; } #endif /* !REPLACE_DUP2 */ diff --git a/lib/dup3.c b/lib/dup3.c index f730e81..3d6f940 100644 --- a/lib/dup3.c +++ b/lib/dup3.c @@ -63,6 +63,10 @@ dup3 (int oldfd, int newfd, int flags) if (!(result < 0 && errno == ENOSYS)) { have_dup3_really = 1; +#ifdef FCHDIR_REPLACEMENT + if (0 <= result) + result = _gl_register_dup (oldfd, newfd); +#endif return result; } have_dup3_really = -1; @@ -180,6 +184,10 @@ dup3 (int oldfd, int newfd, int flags) errno = saved_errno; } +#ifdef FCHDIR_REPLACEMENT + if (result == newfd) + result = _gl_register_dup (oldfd, newfd); +#endif return result; } @@ -218,5 +226,8 @@ dup3 (int oldfd, int newfd, int flags) setmode (newfd, O_TEXT); #endif +#ifdef FCHDIR_REPLACEMENT + newfd = _gl_register_dup (oldfd, newfd); +#endif return newfd; } diff --git a/lib/fchdir.c b/lib/fchdir.c index 170505f..9b64e02 100644 --- a/lib/fchdir.c +++ b/lib/fchdir.c @@ -19,10 +19,11 @@ /* Specification. */ #include <unistd.h> +#include <assert.h> #include <dirent.h> #include <errno.h> #include <fcntl.h> -#include <stdarg.h> +#include <stdbool.h> #include <stdlib.h> #include <string.h> #include <sys/types.h> @@ -43,44 +44,40 @@ object. */ /* Array of file descriptors opened. If it points to a directory, it stores - info about this directory; otherwise it stores an errno value of ENOTDIR. */ + info about this directory. */ typedef struct { char *name; /* Absolute name of the directory, or NULL. */ - int saved_errno; /* If name == NULL: The error code describing the failure - reason. */ + /* FIXME - add a DIR* member to make dirfd possible on mingw? */ } dir_info_t; static dir_info_t *dirs; static size_t dirs_allocated; -/* Try to ensure dirs has enough room for a slot at index fd. */ -static void +/* Try to ensure dirs has enough room for a slot at index fd. Return + false and set errno to ENOMEM on allocation failure. */ +static bool ensure_dirs_slot (size_t fd) { if (fd >= dirs_allocated) { size_t new_allocated; dir_info_t *new_dirs; - size_t i; new_allocated = 2 * dirs_allocated + 1; if (new_allocated <= fd) new_allocated = fd + 1; new_dirs = (dirs != NULL - ? (dir_info_t *) realloc (dirs, new_allocated * sizeof (dir_info_t)) - : (dir_info_t *) malloc (new_allocated * sizeof (dir_info_t))); - if (new_dirs != NULL) - { - for (i = dirs_allocated; i < new_allocated; i++) - { - new_dirs[i].name = NULL; - new_dirs[i].saved_errno = ENOTDIR; - } - dirs = new_dirs; - dirs_allocated = new_allocated; - } + ? (dir_info_t *) realloc (dirs, new_allocated * sizeof *dirs) + : (dir_info_t *) malloc (new_allocated * sizeof *dirs)); + if (new_dirs == NULL) + return false; + memset (new_dirs + dirs_allocated, 0, + (new_allocated - dirs_allocated) * sizeof *dirs); + dirs = new_dirs; + dirs_allocated = new_allocated; } + return true; } /* Hook into the gnulib replacements for open() and close() to keep track @@ -95,27 +92,66 @@ _gl_unregister_fd (int fd) { free (dirs[fd].name); dirs[fd].name = NULL; - dirs[fd].saved_errno = ENOTDIR; } } -/* Mark FD as visiting FILENAME. FD must be positive, and refer to an - open file descriptor. If REPLACE_OPEN_DIRECTORY is non-zero, this - should only be called if FD is visiting a directory. */ -void +/* Mark FD as visiting FILENAME. FD must be non-negative, and refer + to an open file descriptor. If REPLACE_OPEN_DIRECTORY is non-zero, + this should only be called if FD is visiting a directory. Close FD + and return -1 if there is insufficient memory to track the + directory name; otherwise return FD. */ +int _gl_register_fd (int fd, const char *filename) { struct stat statbuf; - ensure_dirs_slot (fd); - if (fd < dirs_allocated - && (REPLACE_OPEN_DIRECTORY - || (fstat (fd, &statbuf) >= 0 && S_ISDIR (statbuf.st_mode)))) + assert (0 <= fd); + if (REPLACE_OPEN_DIRECTORY + || (fstat (fd, &statbuf) == 0 && S_ISDIR (statbuf.st_mode))) + { + if (!ensure_dirs_slot (fd) + || (dirs[fd].name = canonicalize_file_name (filename)) == NULL) + { + int saved_errno = errno; + close (fd); + errno = saved_errno; + return -1; + } + } + return fd; +} + +/* Mark NEWFD as a duplicate of OLDFD; useful from dup, dup2, dup3, + 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. */ +int +_gl_register_dup (int oldfd, int newfd) +{ + assert (0 <= oldfd && 0 <= newfd && oldfd != newfd); + if (oldfd < dirs_allocated && dirs[oldfd].name) { - dirs[fd].name = canonicalize_file_name (filename); - if (dirs[fd].name == NULL) - dirs[fd].saved_errno = errno; + /* Duplicated a directory; must ensure newfd is allocated. */ + if (!ensure_dirs_slot (newfd) + || (dirs[newfd].name = strdup (dirs[oldfd].name)) == NULL) + { + int saved_errno = errno; + close (newfd); + errno = saved_errno; + newfd = -1; + } } + else if (newfd < dirs_allocated) + { + /* Duplicated a non-directory; ensure newfd is cleared. */ + free (dirs[newfd].name); + dirs[newfd].name = NULL; + } + return newfd; } /* Return stat information about FD in STATBUF. Needed when @@ -156,13 +192,18 @@ rpl_opendir (const char *filename) if (dp != NULL) { int fd = dirfd (dp); - if (fd >= 0) - _gl_register_fd (fd, filename); + if (0 <= fd && _gl_register_fd (fd, filename) != fd) + { + int saved_errno = errno; + closedir (dp); + errno = saved_errno; + return NULL; + } } return dp; } -/* Override dup() and dup2(), to keep track of open file descriptors. */ +/* Override dup(), to keep track of open file descriptors. */ int rpl_dup (int oldfd) @@ -170,75 +211,11 @@ rpl_dup (int oldfd) { int newfd = dup (oldfd); - if (oldfd >= 0 && newfd >= 0) - { - ensure_dirs_slot (newfd); - if (newfd < dirs_allocated) - { - if (oldfd < dirs_allocated) - { - if (dirs[oldfd].name != NULL) - { - dirs[newfd].name = strdup (dirs[oldfd].name); - if (dirs[newfd].name == NULL) - dirs[newfd].saved_errno = ENOMEM; - } - else - { - dirs[newfd].name = NULL; - dirs[newfd].saved_errno = dirs[oldfd].saved_errno; - } - } - else - { - dirs[newfd].name = NULL; - dirs[newfd].saved_errno = ENOMEM; - } - } - } + if (0 <= newfd) + newfd = _gl_register_dup (oldfd, newfd); return newfd; } -/* Our <unistd.h> replacement overrides dup2 twice; be sure to pick - the one we want. */ -#if REPLACE_DUP2 -# undef dup2 -# define dup2 rpl_dup2 -#endif - -int -rpl_dup2_fchdir (int oldfd, int newfd) -{ - int retval = dup2 (oldfd, newfd); - - if (retval >= 0 && newfd != oldfd) - { - ensure_dirs_slot (newfd); - if (newfd < dirs_allocated) - { - if (oldfd < dirs_allocated) - { - if (dirs[oldfd].name != NULL) - { - dirs[newfd].name = strdup (dirs[oldfd].name); - if (dirs[newfd].name == NULL) - dirs[newfd].saved_errno = ENOMEM; - } - else - { - dirs[newfd].name = NULL; - dirs[newfd].saved_errno = dirs[oldfd].saved_errno; - } - } - else - { - dirs[newfd].name = NULL; - dirs[newfd].saved_errno = ENOMEM; - } - } - } - return retval; -} /* Implement fchdir() in terms of chdir(). */ diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h index 8b92521..959be44 100644 --- a/lib/fcntl.in.h +++ b/lib/fcntl.in.h @@ -60,7 +60,7 @@ extern int open (const char *filename, int flags, ...); #ifdef FCHDIR_REPLACEMENT /* gnulib internal function. */ -extern void _gl_register_fd (int fd, const char *filename); +extern int _gl_register_fd (int fd, const char *filename); #endif #ifdef __cplusplus diff --git a/lib/open.c b/lib/open.c index 5cfef1f..02dd12d 100644 --- a/lib/open.c +++ b/lib/open.c @@ -118,7 +118,7 @@ open (const char *filename, int flags, ...) /* Maximum recursion depth of 1. */ fd = open ("/dev/null", flags, mode); if (0 <= fd) - _gl_register_fd (fd, filename); + fd = _gl_register_fd (fd, filename); } else errno = EACCES; @@ -157,7 +157,7 @@ open (const char *filename, int flags, ...) #ifdef FCHDIR_REPLACEMENT if (!REPLACE_OPEN_DIRECTORY && 0 <= fd) - _gl_register_fd (fd, filename); + fd = _gl_register_fd (fd, filename); #endif return fd; diff --git a/lib/unistd.in.h b/lib/unistd.in.h index e81d35c..f0b5cc4 100644 --- a/lib/unistd.in.h +++ b/lib/unistd.in.h @@ -248,12 +248,6 @@ extern int fchdir (int /*fd*/); # define dup rpl_dup extern int dup (int); -# if @REPLACE_DUP2@ -# undef dup2 -# endif -# define dup2 rpl_dup2_fchdir -extern int dup2 (int, int); - # endif #elif defined GNULIB_POSIXCHECK # undef fchdir @@ -624,6 +618,8 @@ extern ssize_t write (int fd, const void *buf, size_t count); #ifdef FCHDIR_REPLACEMENT /* gnulib internal function. */ extern void _gl_unregister_fd (int fd); +/* gnulib internal function. */ +extern int _gl_register_dup (int oldfd, int newfd); #endif diff --git a/m4/dup2.m4 b/m4/dup2.m4 index 2e846c0..816a734 100644 --- a/m4/dup2.m4 +++ b/m4/dup2.m4 @@ -1,4 +1,4 @@ -#serial 7 +#serial 8 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, @@ -37,10 +37,16 @@ AC_DEFUN([gl_FUNC_DUP2], esac]) ]) if test "$gl_cv_func_dup2_works" = no; then - REPLACE_DUP2=1 - AC_LIBOBJ([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 + AC_LIBOBJ([dup2]) +]) diff --git a/m4/fchdir.m4 b/m4/fchdir.m4 index 49e6634..bcaf056 100644 --- a/m4/fchdir.m4 +++ b/m4/fchdir.m4 @@ -1,4 +1,4 @@ -# fchdir.m4 serial 8 +# fchdir.m4 serial 9 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, @@ -17,6 +17,8 @@ AC_DEFUN([gl_FUNC_FCHDIR], [Define if gnulib's fchdir() replacement is used.]) gl_REPLACE_OPEN gl_REPLACE_CLOSE + gl_REPLACE_DUP2 + dnl dup3 is already unconditionally replaced gl_REPLACE_DIRENT_H AC_CACHE_CHECK([whether open can visit directories], [gl_cv_func_open_directory_works], diff --git a/modules/fchdir b/modules/fchdir index d3fe0e5..8a1cd1c 100644 --- a/modules/fchdir +++ b/modules/fchdir @@ -13,8 +13,11 @@ dirfd dup2 fcntl-h include_next +malloc-posix open -strdup +realloc-posix +stdbool +strdup-posix sys_stat unistd -- 1.6.3.2 >From 722686c014ecaf829658f3475430b0a6946cd0dc Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Tue, 1 Sep 2009 09:18:16 -0600 Subject: [PATCH 3/5] fchdir: use more consistent macro convention * lib/fcntl.in.h (_gl_register_fd): Move declaration to unistd. * lib/sys_stat.in.h (rpl_fstat): Declare via make-time REPLACE_FCHDIR, rather than relying on config.h macros. * lib/unistd.in.h (fchdir): Move all fchdir internal declarations inside a single make-time REPLACE_FCHDIR block, rather than using the config.h FCHDIR_REPLACEMENT. * m4/fchdir.m4 (gl_FUNC_FCHDIR): REPLACE_FCHDIR was already AC_SUBST'd, also AC_DEFINE it. Don't define FCHDIR_REPLACEMENT. Manage fstat replacement. * m4/sys_stat_h.m4 (gl_SYS_STAT_H_DEFAULTS): Pick up REPLACE_FCHDIR. * modules/sys_stat (Files): Add m4/unistd_h.m4. (Makefile.am): Substitute REPLACE_FCHDIR. * lib/close.c (rpl_close): Use REPLACE_FCHDIR, not FCHDIR_REPLACEMENT. * lib/dup-safer.c (dup_safer): Likewise. * lib/dup2.c (rpl_dup2): Likewise. * lib/dup3.c (rpl_dup3): Likewise. * lib/open.c (rpl_open): Likewise. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 21 +++++++++++++++++++++ lib/close.c | 2 +- lib/dup-safer.c | 2 +- lib/dup2.c | 4 ++-- lib/dup3.c | 6 +++--- lib/fcntl.in.h | 5 ----- lib/open.c | 5 +++-- lib/sys_stat.in.h | 2 +- lib/unistd.in.h | 13 +++++-------- m4/fchdir.m4 | 6 ++++-- m4/sys_stat_h.m4 | 5 +++-- modules/sys_stat | 2 ++ 12 files changed, 46 insertions(+), 27 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6a0b18e..98135f5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,26 @@ 2009-09-01 Eric Blake <e...@byu.net> + fchdir: use more consistent macro convention + * lib/fcntl.in.h (_gl_register_fd): Move declaration to unistd. + * lib/sys_stat.in.h (rpl_fstat): Declare via make-time + REPLACE_FCHDIR, rather than relying on config.h macros. + * lib/unistd.in.h (fchdir): Move all fchdir internal declarations + inside a single make-time REPLACE_FCHDIR block, rather than using + the config.h FCHDIR_REPLACEMENT. + * m4/fchdir.m4 (gl_FUNC_FCHDIR): REPLACE_FCHDIR was already + AC_SUBST'd, also AC_DEFINE it. Don't define FCHDIR_REPLACEMENT. + Manage fstat replacement. + * m4/sys_stat_h.m4 (gl_SYS_STAT_H_DEFAULTS): Pick up + REPLACE_FCHDIR. + * modules/sys_stat (Files): Add m4/unistd_h.m4. + (Makefile.am): Substitute REPLACE_FCHDIR. + * lib/close.c (rpl_close): Use REPLACE_FCHDIR, not + FCHDIR_REPLACEMENT. + * lib/dup-safer.c (dup_safer): Likewise. + * lib/dup2.c (rpl_dup2): Likewise. + * lib/dup3.c (rpl_dup3): Likewise. + * lib/open.c (rpl_open): Likewise. + fchdir: simplify error handling, and support dup3 * modules/fchdir (Depends-on): Use strdup-posix, not strdup. Add stdbool, malloc-posix, realloc-posix. diff --git a/lib/close.c b/lib/close.c index 0e56dcb..5278f24 100644 --- a/lib/close.c +++ b/lib/close.c @@ -33,7 +33,7 @@ rpl_close (int fd) int retval = close (fd); #endif -#ifdef FCHDIR_REPLACEMENT +#if REPLACE_FCHDIR if (retval >= 0) _gl_unregister_fd (fd); #endif diff --git a/lib/dup-safer.c b/lib/dup-safer.c index 7d9b2be..22b96ba 100644 --- a/lib/dup-safer.c +++ b/lib/dup-safer.c @@ -32,7 +32,7 @@ int dup_safer (int fd) { -#if defined F_DUPFD && !defined FCHDIR_REPLACEMENT +#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 diff --git a/lib/dup2.c b/lib/dup2.c index a513e5b..140af1b 100644 --- a/lib/dup2.c +++ b/lib/dup2.c @@ -70,7 +70,7 @@ rpl_dup2 (int fd, int desired_fd) /* Correct a cygwin 1.5.x errno value. */ else if (result == -1 && errno == EMFILE) errno = EBADF; -#ifdef FCHDIR_REPLACEMENT +#if REPLACE_FCHDIR if (fd != desired_fd && result == desired_fd) result = _gl_register_dup (fd, desired_fd); #endif @@ -111,7 +111,7 @@ dup2 (int fd, int desired_fd) # else result = dupfd (fd, desired_fd); # endif -#ifdef FCHDIR_REPLACEMENT +#if REPLACE_FCHDIR if (0 <= result) result = _gl_register_dup (fd, desired_fd); #endif diff --git a/lib/dup3.c b/lib/dup3.c index 3d6f940..879a907 100644 --- a/lib/dup3.c +++ b/lib/dup3.c @@ -63,7 +63,7 @@ dup3 (int oldfd, int newfd, int flags) if (!(result < 0 && errno == ENOSYS)) { have_dup3_really = 1; -#ifdef FCHDIR_REPLACEMENT +#if REPLACE_FCHDIR if (0 <= result) result = _gl_register_dup (oldfd, newfd); #endif @@ -184,7 +184,7 @@ dup3 (int oldfd, int newfd, int flags) errno = saved_errno; } -#ifdef FCHDIR_REPLACEMENT +#if REPLACE_FCHDIR if (result == newfd) result = _gl_register_dup (oldfd, newfd); #endif @@ -226,7 +226,7 @@ dup3 (int oldfd, int newfd, int flags) setmode (newfd, O_TEXT); #endif -#ifdef FCHDIR_REPLACEMENT +#if REPLACE_FCHDIR newfd = _gl_register_dup (oldfd, newfd); #endif return newfd; diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h index 959be44..5c63afd 100644 --- a/lib/fcntl.in.h +++ b/lib/fcntl.in.h @@ -58,11 +58,6 @@ extern int open (const char *filename, int flags, ...); # endif #endif -#ifdef FCHDIR_REPLACEMENT -/* gnulib internal function. */ -extern int _gl_register_fd (int fd, const char *filename); -#endif - #ifdef __cplusplus } #endif diff --git a/lib/open.c b/lib/open.c index 02dd12d..08ecaff 100644 --- a/lib/open.c +++ b/lib/open.c @@ -38,6 +38,7 @@ orig_open (const char *filename, int flags, mode_t mode) #include <string.h> #include <sys/types.h> #include <sys/stat.h> +#include <unistd.h> #ifndef REPLACE_OPEN_DIRECTORY # define REPLACE_OPEN_DIRECTORY 0 @@ -102,7 +103,7 @@ open (const char *filename, int flags, ...) fd = orig_open (filename, flags, mode); -#ifdef FCHDIR_REPLACEMENT +#if REPLACE_FCHDIR /* Implementing fchdir and fdopendir requires the ability to open a directory file descriptor. If open doesn't support that (as on mingw), we use a dummy file that behaves the same as directories @@ -155,7 +156,7 @@ open (const char *filename, int flags, ...) } #endif -#ifdef FCHDIR_REPLACEMENT +#if REPLACE_FCHDIR if (!REPLACE_OPEN_DIRECTORY && 0 <= fd) fd = _gl_register_fd (fd, filename); #endif diff --git a/lib/sys_stat.in.h b/lib/sys_stat.in.h index 9d511da..b034168 100644 --- a/lib/sys_stat.in.h +++ b/lib/sys_stat.in.h @@ -302,7 +302,7 @@ extern int rpl_lstat (const char *name, struct stat *buf); lstat (p, b)) #endif -#if defined FCHDIR_REPLACEMENT && REPLACE_OPEN_DIRECTORY +#if @REPLACE_FCHDIR@ # define fstat rpl_fstat extern int fstat (int fd, struct stat *buf); #endif diff --git a/lib/unistd.in.h b/lib/unistd.in.h index f0b5cc4..3a748a6 100644 --- a/lib/unistd.in.h +++ b/lib/unistd.in.h @@ -248,6 +248,11 @@ extern int fchdir (int /*fd*/); # define dup rpl_dup extern int dup (int); +/* Gnulib internal hooks needed to maintain the fchdir metadata. */ +extern int _gl_register_fd (int fd, const char *filename); +extern void _gl_unregister_fd (int fd); +extern int _gl_register_dup (int oldfd, int newfd); + # endif #elif defined GNULIB_POSIXCHECK # undef fchdir @@ -615,14 +620,6 @@ extern ssize_t write (int fd, const void *buf, size_t count); #endif -#ifdef FCHDIR_REPLACEMENT -/* gnulib internal function. */ -extern void _gl_unregister_fd (int fd); -/* gnulib internal function. */ -extern int _gl_register_dup (int oldfd, int newfd); -#endif - - #ifdef __cplusplus } #endif diff --git a/m4/fchdir.m4 b/m4/fchdir.m4 index bcaf056..6597be0 100644 --- a/m4/fchdir.m4 +++ b/m4/fchdir.m4 @@ -8,13 +8,15 @@ AC_DEFUN([gl_FUNC_FCHDIR], [ AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) AC_REQUIRE([gl_DIRENT_H_DEFAULTS]) + AC_REQUIRE([gl_SYS_STAT_H_DEFAULTS]) AC_CHECK_FUNCS_ONCE([fchdir]) if test $ac_cv_func_fchdir = no; then REPLACE_FCHDIR=1 AC_LIBOBJ([fchdir]) gl_PREREQ_FCHDIR - AC_DEFINE([FCHDIR_REPLACEMENT], [1], - [Define if gnulib's fchdir() replacement is used.]) + AC_DEFINE([REPLACE_FCHDIR], [1], + [Define to 1 if gnulib's fchdir() replacement is used.]) + REPLACE_FSTAT=1 gl_REPLACE_OPEN gl_REPLACE_CLOSE gl_REPLACE_DUP2 diff --git a/m4/sys_stat_h.m4 b/m4/sys_stat_h.m4 index 20c82ed..5113e55 100644 --- a/m4/sys_stat_h.m4 +++ b/m4/sys_stat_h.m4 @@ -1,5 +1,5 @@ -# sys_stat_h.m4 serial 10 -*- Autoconf -*- -dnl Copyright (C) 2006-2008 Free Software Foundation, Inc. +# sys_stat_h.m4 serial 11 -*- Autoconf -*- +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, dnl with or without modifications, as long as this notice is preserved. @@ -50,6 +50,7 @@ AC_DEFUN([gl_SYS_STAT_MODULE_INDICATOR], AC_DEFUN([gl_SYS_STAT_H_DEFAULTS], [ + AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) dnl for REPLACE_FCHDIR GNULIB_LCHMOD=0; AC_SUBST([GNULIB_LCHMOD]) GNULIB_LSTAT=0; AC_SUBST([GNULIB_LSTAT]) dnl Assume proper GNU behavior unless another module says otherwise. diff --git a/modules/sys_stat b/modules/sys_stat index e4fba73..40f1c18 100644 --- a/modules/sys_stat +++ b/modules/sys_stat @@ -4,6 +4,7 @@ A <sys/stat.h> for systems with missing declarations. Files: lib/sys_stat.in.h m4/sys_stat_h.m4 +m4/unistd_h.m4 Depends-on: include_next @@ -31,6 +32,7 @@ sys/stat.h: sys_stat.in.h -e 's|@''HAVE_LSTAT''@|$(HAVE_LSTAT)|g' \ -e 's|@''REPLACE_LSTAT''@|$(REPLACE_LSTAT)|g' \ -e 's|@''REPLACE_MKDIR''@|$(REPLACE_MKDIR)|g' \ + -e 's|@''REPLACE_FCHDIR''@|$(REPLACE_FCHDIR)|g' \ -e '/definition of GL_LINK_WARNING/r $(LINK_WARNING_H)' \ < $(srcdir)/sys_stat.in.h; \ } > $...@-t && \ -- 1.6.3.2 >From ecedd972c813d7d9ecf0308373b620fc7dd507ea Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Mon, 31 Aug 2009 20:37:59 -0600 Subject: [PATCH 4/5] fdopendir: split into its own module * lib/openat.c (fdopendir): Move... * lib/fdopendir.c: ...into new file. * modules/fdopendir: New module. * m4/fdopendir.m4 (gl_FUNC_FDOPENDIR): New file. * modules/openat (Depends-on): Add fdopendir. * m4/openat.m4 (gl_FUNC_OPENAT): No longer need to check for fdopendir here. * modules/savedir (Depends-on): Only need fdopendir, not full openat. * lib/savedir.c (include): Use <dirent.h>, not "openat.h". * lib/openat.h (fdopendir): Drop prototype. * lib/dirent.in.h (fdopendir): Provide prototype. * m4/dirent_h.m4 (gl_DIRENT_H_DEFAULTS): Add replacements. * modules/dirent (Makefile.am): Substitute them. * MODULES.html.sh (File system functions): Mention it. * doc/posix-functions/fdopendir.texi (fdopendir): Likewise. * modules/fdopendir-tests: New file. * tests/test-fdopendir.c: Likewise. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 20 ++++++++ MODULES.html.sh | 1 + doc/posix-functions/fdopendir.texi | 7 ++- lib/dirent.in.h | 17 ++++++ lib/fdopendir.c | 95 ++++++++++++++++++++++++++++++++++++ lib/openat.c | 68 ------------------------- lib/openat.h | 6 +-- lib/savedir.c | 3 +- m4/dirent_h.m4 | 10 ++-- m4/fdopendir.m4 | 21 ++++++++ m4/openat.m4 | 3 +- modules/dirent | 2 + modules/fdopendir | 30 +++++++++++ modules/fdopendir-tests | 13 +++++ modules/openat | 1 + modules/savedir | 2 +- tests/test-fdopendir.c | 76 ++++++++++++++++++++++++++++ 17 files changed, 291 insertions(+), 84 deletions(-) create mode 100644 lib/fdopendir.c create mode 100644 m4/fdopendir.m4 create mode 100644 modules/fdopendir create mode 100644 modules/fdopendir-tests create mode 100644 tests/test-fdopendir.c diff --git a/ChangeLog b/ChangeLog index 98135f5..9d0dbef 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,25 @@ 2009-09-01 Eric Blake <e...@byu.net> + fdopendir: split into its own module + * lib/openat.c (fdopendir): Move... + * lib/fdopendir.c: ...into new file. + * modules/fdopendir: New module. + * m4/fdopendir.m4 (gl_FUNC_FDOPENDIR): New file. + * modules/openat (Depends-on): Add fdopendir. + * m4/openat.m4 (gl_FUNC_OPENAT): No longer need to check for + fdopendir here. + * modules/savedir (Depends-on): Only need fdopendir, not full + openat. + * lib/savedir.c (include): Use <dirent.h>, not "openat.h". + * lib/openat.h (fdopendir): Drop prototype. + * lib/dirent.in.h (fdopendir): Provide prototype. + * m4/dirent_h.m4 (gl_DIRENT_H_DEFAULTS): Add replacements. + * modules/dirent (Makefile.am): Substitute them. + * MODULES.html.sh (File system functions): Mention it. + * doc/posix-functions/fdopendir.texi (fdopendir): Likewise. + * modules/fdopendir-tests: New file. + * tests/test-fdopendir.c: Likewise. + fchdir: use more consistent macro convention * lib/fcntl.in.h (_gl_register_fd): Move declaration to unistd. * lib/sys_stat.in.h (rpl_fstat): Declare via make-time diff --git a/MODULES.html.sh b/MODULES.html.sh index 3c54cec..027a0bc 100755 --- a/MODULES.html.sh +++ b/MODULES.html.sh @@ -2451,6 +2451,7 @@ func_all_modules () func_module dirfd func_module double-slash-root func_module euidaccess + func_module fdopendir func_module file-type func_module fileblocks func_module filemode diff --git a/doc/posix-functions/fdopendir.texi b/doc/posix- functions/fdopendir.texi index 1d9827c..20db12c 100644 --- a/doc/posix-functions/fdopendir.texi +++ b/doc/posix-functions/fdopendir.texi @@ -4,7 +4,7 @@ fdopendir POSIX specification: @url {http://www.opengroup.org/onlinepubs/9699919799/functions/fdopendir.html} -Gnulib module: openat +Gnulib module: fdopendir Portability problems fixed by Gnulib: @itemize @@ -12,7 +12,10 @@ fdopendir This function is missing on some platforms: glibc 2.3.6, MacOS X 10.3, FreeBSD 6.0, NetBSD 3.0, OpenBSD 3.8, AIX 5.1, HP-UX 11, IRIX 6.5, OSF/1 5.1, Cygwin 1.5.x, mingw, Interix 3.5, BeOS. -But the replacement function is not safe to be used in libraries and is not multithread-safe. +But the replacement function is not safe to be used in libraries and +is not multithread-safe. Also, the replacement does not guarantee +that @samp{dirfd(fdopendir(n))==n} (dirfd might fail, or return a +different file descriptor than n). @end itemize Portability problems not fixed by Gnulib: diff --git a/lib/dirent.in.h b/lib/dirent.in.h index 15f0245..8930765 100644 --- a/lib/dirent.in.h +++ b/lib/dirent.in.h @@ -58,6 +58,23 @@ extern int dirfd (DIR const *dir); dirfd (d)) #endif +#if @GNULIB_FDOPENDIR@ +# if !...@have_fdopendir@ +/* Open a directory stream visiting the given directory file + descriptor. Return NULL and set errno if fd is not visiting a + directory. On success, this function consumes fd (it will be + implicitly closed either by this function or by a subsequent + closedir). */ +extern DIR *fdopendir (int fd); +# endif +#elif defined GNULIB_POSIXCHECK +# undef fdopendir +# define fdopendir(f) \ + (GL_LINK_WARNING ("fdopendir is unportable - " \ + "use gnulib module fdopendir for portability"), \ + fdopendir (f)) +#endif + #if @GNULIB_SCANDIR@ /* Scan the directory DIR, calling FILTER on each directory entry. Entries for which FILTER returns nonzero are individually malloc'd, diff --git a/lib/fdopendir.c b/lib/fdopendir.c new file mode 100644 index 0000000..7095156 --- /dev/null +++ b/lib/fdopendir.c @@ -0,0 +1,95 @@ +/* provide a replacement fdopendir function + Copyright (C) 2004-2009 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* written by Jim Meyering */ + +#include <config.h> + +#include <dirent.h> + +#include <stdlib.h> +#include <unistd.h> + +#include "openat.h" +#include "openat-priv.h" +#include "save-cwd.h" + +/* 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 + that, simulate it by doing 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. + 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. */ +DIR * +fdopendir (int fd) +{ + struct saved_cwd saved_cwd; + int saved_errno; + DIR *dir; + + char buf[OPENAT_BUFFER_SIZE]; + char *proc_file = openat_proc_name (buf, fd, "."); + if (proc_file) + { + dir = opendir (proc_file); + saved_errno = errno; + } + else + { + dir = NULL; + saved_errno = EOPNOTSUPP; + } + + /* If the syscall fails with an expected errno value, resort to + save_cwd/restore_cwd. */ + if (! dir && EXPECTED_ERRNO (saved_errno)) + { + if (save_cwd (&saved_cwd) != 0) + openat_save_fail (errno); + + if (fchdir (fd) != 0) + { + dir = NULL; + saved_errno = errno; + } + else + { + dir = opendir ("."); + saved_errno = errno; + + if (restore_cwd (&saved_cwd) != 0) + openat_restore_fail (errno); + } + + free_cwd (&saved_cwd); + } + + if (dir) + close (fd); + if (proc_file != buf) + free (proc_file); + errno = saved_errno; + return dir; +} diff --git a/lib/openat.c b/lib/openat.c index 77a85bf..7a68cba 100644 --- a/lib/openat.c +++ b/lib/openat.c @@ -152,74 +152,6 @@ openat_needs_fchdir (void) return needs_fchdir; } -#if !HAVE_FDOPENDIR - -/* 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 - that, simulate it by doing 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. - Otherwise, this function works just like Solaris' fdopendir. - - 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 if this function returns successfully. */ -DIR * -fdopendir (int fd) -{ - struct saved_cwd saved_cwd; - int saved_errno; - DIR *dir; - - char buf[OPENAT_BUFFER_SIZE]; - char *proc_file = openat_proc_name (buf, fd, "."); - if (proc_file) - { - dir = opendir (proc_file); - saved_errno = errno; - } - else - { - dir = NULL; - saved_errno = EOPNOTSUPP; - } - - /* If the syscall fails with an expected errno value, resort to - save_cwd/restore_cwd. */ - if (! dir && EXPECTED_ERRNO (saved_errno)) - { - if (save_cwd (&saved_cwd) != 0) - openat_save_fail (errno); - - if (fchdir (fd) != 0) - { - dir = NULL; - saved_errno = errno; - } - else - { - dir = opendir ("."); - saved_errno = errno; - - if (restore_cwd (&saved_cwd) != 0) - openat_restore_fail (errno); - } - - free_cwd (&saved_cwd); - } - - if (dir) - close (fd); - if (proc_file != buf) - free (proc_file); - 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> First, try to simulate it via l?stat ("/proc/self/fd/FD/FILE"). diff --git a/lib/openat.h b/lib/openat.h index 68c7df0..4072c94 100644 --- a/lib/openat.h +++ b/lib/openat.h @@ -1,5 +1,5 @@ /* provide a replacement openat function - Copyright (C) 2004, 2005, 2006, 2008 Free Software Foundation, Inc. + Copyright (C) 2004-2006, 2008-2009 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -66,10 +66,6 @@ int openat (int fd, char const *file, int flags, /* mode_t mode */ ...); int openat_permissive (int fd, char const *file, int flags, mode_t mode, int *cwd_errno); -# 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); # define unlinkat __OPENAT_ID (unlinkat) diff --git a/lib/savedir.c b/lib/savedir.c index b837414..8400145 100644 --- a/lib/savedir.c +++ b/lib/savedir.c @@ -1,7 +1,7 @@ /* savedir.c -- save the list of files in a directory in a string Copyright (C) 1990, 1997, 1998, 1999, 2000, 2001, 2003, 2004, 2005, - 2006 Free Software Foundation, Inc. + 2006, 2009 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -35,7 +35,6 @@ #include <stdlib.h> #include <string.h> -#include "openat.h" #include "xalloc.h" #ifndef NAME_SIZE_DEFAULT diff --git a/m4/dirent_h.m4 b/m4/dirent_h.m4 index e507e3d..06fffef 100644 --- a/m4/dirent_h.m4 +++ b/m4/dirent_h.m4 @@ -33,11 +33,13 @@ AC_DEFUN([gl_DIRENT_H_DEFAULTS], [ AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) dnl for REPLACE_FCHDIR GNULIB_DIRFD=0; AC_SUBST([GNULIB_DIRFD]) + GNULIB_FDOPENDIR=0; AC_SUBST([GNULIB_FDOPENDIR]) GNULIB_SCANDIR=0; AC_SUBST([GNULIB_SCANDIR]) GNULIB_ALPHASORT=0; AC_SUBST([GNULIB_ALPHASORT]) dnl Assume proper GNU behavior unless another module says otherwise. - HAVE_DECL_DIRFD=1; AC_SUBST([HAVE_DECL_DIRFD]) - HAVE_SCANDIR=1; AC_SUBST([HAVE_SCANDIR]) - HAVE_ALPHASORT=1; AC_SUBST([HAVE_ALPHASORT]) - DIRENT_H=''; AC_SUBST([DIRENT_H]) + HAVE_DECL_DIRFD=1; AC_SUBST([HAVE_DECL_DIRFD]) + HAVE_FDOPENDIR=1; AC_SUBST([HAVE_FDOPENDIR]) + HAVE_SCANDIR=1; AC_SUBST([HAVE_SCANDIR]) + HAVE_ALPHASORT=1; AC_SUBST([HAVE_ALPHASORT]) + DIRENT_H=''; AC_SUBST([DIRENT_H]) ]) diff --git a/m4/fdopendir.m4 b/m4/fdopendir.m4 new file mode 100644 index 0000000..09670bb --- /dev/null +++ b/m4/fdopendir.m4 @@ -0,0 +1,21 @@ +# serial 1 +# See if we need to provide fdopendir. + +dnl Copyright (C) 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, +dnl with or without modifications, as long as this notice is preserved. + +# Written by Eric Blake. + +AC_DEFUN([gl_FUNC_FDOPENDIR], +[ + AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) + AC_CHECK_FUNCS_ONCE([fdopendir]) + if test $ac_cv_func_fdopendir = no; then + AC_LIBOBJ([openat-proc]) + AC_LIBOBJ([fdopendir]) + gl_REPLACE_DIRENT_H + HAVE_FDOPENDIR=0 + fi +]) diff --git a/m4/openat.m4 b/m4/openat.m4 index daa6a53..c8403a1 100644 --- a/m4/openat.m4 +++ b/m4/openat.m4 @@ -1,4 +1,4 @@ -# serial 18 +# serial 19 # See if we need to use our replacement for Solaris' openat et al functions. dnl Copyright (C) 2004-2009 Free Software Foundation, Inc. @@ -13,7 +13,6 @@ AC_DEFUN([gl_FUNC_OPENAT], AC_LIBOBJ([openat-proc]) AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) AC_CHECK_FUNCS_ONCE([lchmod]) - AC_CHECK_FUNCS_ONCE([fdopendir]) AC_REPLACE_FUNCS([fchmodat mkdirat openat]) AC_REQUIRE([AC_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK]) case $ac_cv_func_openat+$ac_cv_func_lstat_dereferences_slashed_symlink in diff --git a/modules/dirent b/modules/dirent index 8df7c35..3eb7411 100644 --- a/modules/dirent +++ b/modules/dirent @@ -25,9 +25,11 @@ dirent.h: dirent.in.h -e 's|@''PRAGMA_SYSTEM_HEADER''@|@PRAGMA_SYSTEM_HEADER@|g' \ -e 's|@''NEXT_DIRENT_H''@|$(NEXT_DIRENT_H)|g' \ -e 's|@''GNULIB_DIRFD''@|$(GNULIB_DIRFD)|g' \ + -e 's|@''GNULIB_FDOPENDIR''@|$(GNULIB_FDOPENDIR)|g' \ -e 's|@''GNULIB_SCANDIR''@|$(GNULIB_SCANDIR)|g' \ -e 's|@''GNULIB_ALPHASORT''@|$(GNULIB_ALPHASORT)|g' \ -e 's|@''HAVE_DECL_DIRFD''@|$(HAVE_DECL_DIRFD)|g' \ + -e 's|@''HAVE_FDOPENDIR''@|$(HAVE_FDOPENDIR)|g' \ -e 's|@''HAVE_SCANDIR''@|$(HAVE_SCANDIR)|g' \ -e 's|@''HAVE_ALPHASORT''@|$(HAVE_ALPHASORT)|g' \ -e 's|@''REPLACE_FCHDIR''@|$(REPLACE_FCHDIR)|g' \ diff --git a/modules/fdopendir b/modules/fdopendir new file mode 100644 index 0000000..7a90aa3 --- /dev/null +++ b/modules/fdopendir @@ -0,0 +1,30 @@ +Description: +Open a directory stream from a file descriptor. + +Files: +lib/fdopendir.c +lib/openat-priv.h +lib/openat-proc.c +m4/fdopendir.m4 + +Depends-on: +extensions +dirent +fchdir +openat-die +save-cwd + +configure.ac: +gl_FUNC_FDOPENDIR +gl_DIRENT_MODULE_INDICATOR([fdopendir]) + +Makefile.am: + +Include: +<dirent.h> + +License: +GPL + +Maintainer: +Jim Meyering, Eric Blake diff --git a/modules/fdopendir-tests b/modules/fdopendir-tests new file mode 100644 index 0000000..9df5e29 --- /dev/null +++ b/modules/fdopendir-tests @@ -0,0 +1,13 @@ +Files: +tests/test-fdopendir.c + +Depends-on: +open +progname + +configure.ac: + +Makefile.am: +TESTS += test-fdopendir +check_PROGRAMS += test-fdopendir +test_fdopendir_LDADD = $(LDADD) @LIBINTL@ diff --git a/modules/openat b/modules/openat index 561687d..5c326a0 100644 --- a/modules/openat +++ b/modules/openat @@ -18,6 +18,7 @@ Depends-on: dirname extensions fchdir +fdopendir gettext-h intprops lchown diff --git a/modules/savedir b/modules/savedir index e781af7..4171b80 100644 --- a/modules/savedir +++ b/modules/savedir @@ -7,7 +7,7 @@ lib/savedir.c m4/savedir.m4 Depends-on: -openat +fdopendir xalloc configure.ac: diff --git a/tests/test-fdopendir.c b/tests/test-fdopendir.c new file mode 100644 index 0000000..003a279 --- /dev/null +++ b/tests/test-fdopendir.c @@ -0,0 +1,76 @@ +/* Test opening a directory stream from a file descriptor. + Copyright (C) 2009 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* Written by Eric Blake <e...@byu.net>, 2009. */ + +#include <config.h> + +#include <dirent.h> + +#include <errno.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> + +#define ASSERT(expr) \ + do \ + { \ + if (!(expr)) \ + { \ + fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (stderr); \ + abort (); \ + } \ + } \ + while (0) + +int +main () +{ + DIR *d; + int fd; + + /* A non-directory cannot be turned into a directory stream. */ + fd = open ("/dev/null", O_RDONLY); + ASSERT (0 <= fd); + errno = 0; + ASSERT (fdopendir (fd) == NULL); + ASSERT (errno == ENOTDIR); + ASSERT (close (fd) == 0); + + /* A bad fd cannot be turned into a stream. */ + errno = 0; + ASSERT (fdopendir (-1) == NULL); + ASSERT (errno == EBADF); + + /* This should work. */ + fd = open (".", O_RDONLY); + ASSERT (0 <= fd); + d = fdopendir (fd); + /* We know that fd is now out of our reach, but it is not specified + whether it is closed now or at the closedir. We also can't + guarantee whether dirfd returns fd, some other descriptor, or + -1. */ + ASSERT (d); + ASSERT (closedir (d) == 0); + /* Now we can guarantee that fd must be closed. */ + errno = 0; + ASSERT (dup2 (fd, fd) == -1); + ASSERT (errno == EBADF); + + return 0; +} -- 1.6.3.2 >From 14ec1183205e82af76df6589f7fd8766e8b4995d Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Tue, 1 Sep 2009 10:05:44 -0600 Subject: [PATCH 5/5] fdopendir: optimize on mingw * lib/unistd.in.h (_gl_directory_name): New prototype. * lib/fchdir.c (_gl_directory_name): Implement it. (fchdir): Use it to simplify implementation. * lib/fdopendir.c (fdopendir) [REPLACE_FCHDIR]: Use metadata from fchdir, when available, to avoid calling [f]chdir(). Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 7 +++++++ lib/fchdir.c | 33 +++++++++++++++++++++------------ lib/fdopendir.c | 12 ++++++++++-- lib/unistd.in.h | 1 + 4 files changed, 39 insertions(+), 14 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9d0dbef..ed1736a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2009-09-01 Eric Blake <e...@byu.net> + fdopendir: optimize on mingw + * lib/unistd.in.h (_gl_directory_name): New prototype. + * lib/fchdir.c (_gl_directory_name): Implement it. + (fchdir): Use it to simplify implementation. + * lib/fdopendir.c (fdopendir) [REPLACE_FCHDIR]: Use metadata from + fchdir, when available, to avoid calling [f]chdir(). + fdopendir: split into its own module * lib/openat.c (fdopendir): Move... * lib/fdopendir.c: ...into new file. diff --git a/lib/fchdir.c b/lib/fchdir.c index 9b64e02..f355629 100644 --- a/lib/fchdir.c +++ b/lib/fchdir.c @@ -154,6 +154,25 @@ _gl_register_dup (int oldfd, int newfd) return newfd; } +/* If FD is currently visiting a directory, then return the name of + that directory. Otherwise, return NULL and set errno. */ +const char * +_gl_directory_name (int fd) +{ + if (0 <= fd && fd < dirs_allocated && dirs[fd].name != NULL) + return dirs[fd].name; + /* At this point, fd is either invalid, or open but not a directory. + If dup2 fails, errno is correctly EBADF. */ + if (0 <= fd) + { + if (dup2 (fd, fd) == fd) + errno = ENOTDIR; + } + else + errno = EBADF; + return NULL; +} + /* 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. */ @@ -222,16 +241,6 @@ rpl_dup (int oldfd) int fchdir (int fd) { - if (fd >= 0 && fd < dirs_allocated && dirs[fd].name != NULL) - return chdir (dirs[fd].name); - /* At this point, fd is either invalid, or open but not a directory. - If dup2 fails, errno is correctly EBADF. */ - if (0 <= fd) - { - if (dup2 (fd, fd) == fd) - errno = ENOTDIR; - } - else - errno = EBADF; - return -1; + const char *name = _gl_directory_name (fd); + return name ? chdir (name) : -1; } diff --git a/lib/fdopendir.c b/lib/fdopendir.c index 7095156..3bc13ac 100644 --- a/lib/fdopendir.c +++ b/lib/fdopendir.c @@ -30,7 +30,8 @@ /* 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 - that, simulate it by doing save_cwd/fchdir/opendir(".")/restore_cwd. + that, simulate it by using fchdir metadata, or by doing + 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. Otherwise, this function works just like Solaris' fdopendir. @@ -45,7 +46,6 @@ DIR * fdopendir (int fd) { - struct saved_cwd saved_cwd; int saved_errno; DIR *dir; @@ -66,6 +66,13 @@ fdopendir (int fd) save_cwd/restore_cwd. */ if (! dir && EXPECTED_ERRNO (saved_errno)) { +#if REPLACE_FCHDIR + const char *name = _gl_directory_name (fd); + if (name) + dir = opendir (name); + saved_errno = errno; +#else /* !REPLACE_FCHDIR */ + struct saved_cwd saved_cwd; if (save_cwd (&saved_cwd) != 0) openat_save_fail (errno); @@ -84,6 +91,7 @@ fdopendir (int fd) } free_cwd (&saved_cwd); +#endif /* !REPLACE_FCHDIR */ } if (dir) diff --git a/lib/unistd.in.h b/lib/unistd.in.h index 3a748a6..fe645b7 100644 --- a/lib/unistd.in.h +++ b/lib/unistd.in.h @@ -252,6 +252,7 @@ extern int dup (int); extern int _gl_register_fd (int fd, const char *filename); extern void _gl_unregister_fd (int fd); extern int _gl_register_dup (int oldfd, int newfd); +extern const char *_gl_directory_name (int fd); # endif #elif defined GNULIB_POSIXCHECK -- 1.6.3.2