From: Eric Blake Date: Mon, 31 Aug 2009 20:20:03 +0000 (-0600) Subject: fchdir: simplify error handling, and support dup3 X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4475e25b6a19e31e5783781f2132cdbd05bcf7c4;p=pspp 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 --- diff --git a/ChangeLog b/ChangeLog index 23f193e8cf..6a458876e2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,24 @@ +2009-09-02 Eric Blake + + 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. + 2009-09-02 Ralf Wildenhues * gnulib-tool (func_create_testdir, func_create_megatestdir): Use diff --git a/lib/dup2.c b/lib/dup2.c index fb3ffb0df0..a513e5bfea 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 f730e81e9f..3d6f940cd4 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 170505f157..9b64e0294b 100644 --- a/lib/fchdir.c +++ b/lib/fchdir.c @@ -19,10 +19,11 @@ /* Specification. */ #include +#include #include #include #include -#include +#include #include #include #include @@ -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 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 8b92521440..959be44ea6 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 5cfef1fb7f..02dd12d0b9 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 e81d35c675..f0b5cc4b52 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 2e846c0c7f..816a734068 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 49e6634f87..bcaf056c78 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 d3fe0e5384..8a1cd1c1b0 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