Mercurial > hg > octave-lojdl > gnulib-hg
changeset 12259:80f02cc1739e
open: detect FreeBSD bug
open("link-to-file/", O_RDONLY) mistakenly succeeds. The previous
patch was enough to fix utimens when no fd is involved, but this is
necessary for futimens to pass.
* m4/open.m4 (gl_FUNC_OPEN): Also detect FreeBSD bug with slash on
symlink.
* doc/posix-functions/open.texi (open): Document the bug.
* doc/posix-functions/utimes.texi (utimes): Likewise.
* tests/test-open.h (test_open): Add parameters, and test symlink
handling.
* tests/test-open.c (main): Adjust caller.
* tests/test-fcntl-safer.c (main): Likewise.
* modules/open-tests (Depends-on): Add stdbool, symlink.
* modules/fcntl-safer-tests (Depends-on): Likewise.
* tests/test-openat.c (main): Add test-open tests.
Signed-off-by: Eric Blake <ebb9@byu.net>
author | Eric Blake <ebb9@byu.net> |
---|---|
date | Sat, 07 Nov 2009 21:34:32 -0700 |
parents | 150adbe92b27 |
children | 18ce29e54b9f |
files | ChangeLog doc/posix-functions/open.texi doc/posix-functions/utimes.texi m4/open.m4 modules/fcntl-safer-tests modules/open-tests tests/test-fcntl-safer.c tests/test-open.c tests/test-open.h tests/test-openat.c |
diffstat | 10 files changed, 143 insertions(+), 46 deletions(-) [+] |
line wrap: on
line diff
--- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,18 @@ 2009-11-09 Eric Blake <ebb9@byu.net> + open: detect FreeBSD bug + * m4/open.m4 (gl_FUNC_OPEN): Also detect FreeBSD bug with slash on + symlink. + * doc/posix-functions/open.texi (open): Document the bug. + * doc/posix-functions/utimes.texi (utimes): Likewise. + * tests/test-open.h (test_open): Add parameters, and test symlink + handling. + * tests/test-open.c (main): Adjust caller. + * tests/test-fcntl-safer.c (main): Likewise. + * modules/open-tests (Depends-on): Add stdbool, symlink. + * modules/fcntl-safer-tests (Depends-on): Likewise. + * tests/test-openat.c (main): Add test-open tests. + stat: detect FreeBSD bug * m4/stat.m4 (gl_FUNC_STAT): Also detect FreeBSD bug with slash on symlink.
--- a/doc/posix-functions/open.texi +++ b/doc/posix-functions/open.texi @@ -12,7 +12,7 @@ This function does not fail when the file name argument ends in a slash and (without the slash) names a nonexistent file or a file that is not a directory, on some platforms: -HP-UX 11.00, Solaris 9, Irix 5.3. +FreeBSD 7.2, HP-UX 11.00, Solaris 9, Irix 5.3. @item On Windows platforms (excluding Cygwin), this function does usually not recognize the @file{/dev/null} filename.
--- a/doc/posix-functions/utimes.texi +++ b/doc/posix-functions/utimes.texi @@ -17,7 +17,7 @@ mingw, Interix 3.5, BeOS. @item On some platforms, this function mis-handles trailing slash: -Solaris 9. +FreeBSD 7.2, Solaris 9. @item This function cannot set full timestamp resolution. In particular, some platforms incorrectly round rather than truncate. Use
--- a/m4/open.m4 +++ b/m4/open.m4 @@ -1,4 +1,4 @@ -# open.m4 serial 7 +# open.m4 serial 8 dnl Copyright (C) 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, @@ -13,10 +13,15 @@ ;; *) dnl open("foo/") should not create a file when the file name has a - dnl trailing slash. + dnl trailing slash. FreeBSD only has the problem on symlinks. + AC_CHECK_FUNCS_ONCE([lstat]) AC_CACHE_CHECK([whether open recognizes a trailing slash], [gl_cv_func_open_slash], - [ + [# Assume that if we have lstat, we can also check symlinks. + if test $ac_cv_func_lstat = yes; then + touch conftest.tmp + ln -s conftest.tmp conftest.lnk + fi AC_TRY_RUN([ #include <fcntl.h> #if HAVE_UNISTD_H @@ -24,18 +29,22 @@ #endif int main () { +#if HAVE_LSTAT + if (open ("conftest.lnk/", O_RDONLY) != -1) return 2; +#endif return open ("conftest.sl/", O_CREAT, 0600) >= 0; }], [gl_cv_func_open_slash=yes], [gl_cv_func_open_slash=no], [ changequote(,)dnl case "$host_os" in + freebsd*) gl_cv_func_open_slash="guessing no" ;; solaris2.[0-9]*) gl_cv_func_open_slash="guessing no" ;; hpux*) gl_cv_func_open_slash="guessing no" ;; *) gl_cv_func_open_slash="guessing yes" ;; esac changequote([,])dnl ]) - rm -f conftest.sl + rm -f conftest.sl conftest.tmp conftest.lnk ]) case "$gl_cv_func_open_slash" in *no)
--- a/modules/fcntl-safer-tests +++ b/modules/fcntl-safer-tests @@ -3,6 +3,8 @@ tests/test-fcntl-safer.c Depends-on: +stdbool +symlink configure.ac:
--- a/modules/open-tests +++ b/modules/open-tests @@ -3,10 +3,11 @@ tests/test-open.c Depends-on: +stdbool +symlink configure.ac: Makefile.am: TESTS += test-open check_PROGRAMS += test-open -
--- a/tests/test-fcntl-safer.c +++ b/tests/test-fcntl-safer.c @@ -20,6 +20,24 @@ #include "fcntl--.h" +#include <errno.h> +#include <stdbool.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) + #define BASE "test-fcntl-safer.t" #include "test-open.h" @@ -27,5 +45,5 @@ int main (void) { - return test_open (); + return test_open (open, true); }
--- a/tests/test-open.c +++ b/tests/test-open.c @@ -20,6 +20,24 @@ #include <fcntl.h> +#include <errno.h> +#include <stdbool.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) + #define BASE "test-open.t" #include "test-open.h" @@ -27,5 +45,5 @@ int main (void) { - return test_open (); + return test_open (open, true); }
--- a/tests/test-open.h +++ b/tests/test-open.h @@ -16,29 +16,14 @@ /* Written by Bruno Haible <bruno@clisp.org>, 2007. */ -/* Include <config.h> and a form of <fcntl.h> first. */ - -#include <errno.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) - -/* Test fopen. Assumes BASE is defined. */ +/* This file is designed to test both open(n,buf[,mode]) and + openat(AT_FDCWD,n,buf[,mode]). FUNC is the function to test. + Assumes that BASE and ASSERT are already defined, and that + appropriate headers are already included. If PRINT, warn before + skipping symlink tests with status 77. */ static int -test_open (void) +test_open (int (*func) (char const *, int, ...), bool print) { int fd; /* Remove anything from prior partial run. */ @@ -46,40 +31,57 @@ /* Cannot create directory. */ errno = 0; - ASSERT (open ("nonexist.ent/", O_CREAT | O_RDONLY, 0600) == -1); + ASSERT (func ("nonexist.ent/", O_CREAT | O_RDONLY, 0600) == -1); ASSERT (errno == ENOTDIR || errno == EISDIR || errno == ENOENT || errno == EINVAL); /* Create a regular file. */ - fd = open (BASE "file", O_CREAT | O_RDONLY, 0600); + fd = func (BASE "file", O_CREAT | O_RDONLY, 0600); ASSERT (0 <= fd); ASSERT (close (fd) == 0); /* Trailing slash handling. */ errno = 0; - ASSERT (open (BASE "file/", O_RDONLY) == -1); + ASSERT (func (BASE "file/", O_RDONLY) == -1); ASSERT (errno == ENOTDIR || errno == EISDIR || errno == EINVAL); /* Directories cannot be opened for writing. */ errno = 0; - ASSERT (open (".", O_WRONLY) == -1); + ASSERT (func (".", O_WRONLY) == -1); ASSERT (errno == EISDIR || errno == EACCES); /* /dev/null must exist, and be writable. */ - fd = open ("/dev/null", O_RDONLY); + fd = func ("/dev/null", O_RDONLY); ASSERT (0 <= fd); { char c; ASSERT (read (fd, &c, 1) == 0); } ASSERT (close (fd) == 0); - fd = open ("/dev/null", O_WRONLY); + fd = func ("/dev/null", O_WRONLY); ASSERT (0 <= fd); ASSERT (write (fd, "c", 1) == 1); ASSERT (close (fd) == 0); + /* Symlink handling, where supported. */ + if (symlink (BASE "file", BASE "link") != 0) + { + ASSERT (unlink (BASE "file") == 0); + if (print) + fputs ("skipping test: symlinks not supported on this file system\n", + stderr); + return 77; + } + errno = 0; + ASSERT (func (BASE "link/", O_RDONLY) == -1); + ASSERT (errno == ENOTDIR); + fd = func (BASE "link", O_RDONLY); + ASSERT (0 <= fd); + ASSERT (close (fd) == 0); + /* Cleanup. */ ASSERT (unlink (BASE "file") == 0); + ASSERT (unlink (BASE "link") == 0); return 0; }
--- a/tests/test-openat.c +++ b/tests/test-openat.c @@ -20,6 +20,9 @@ #include <fcntl.h> +#include <errno.h> +#include <stdarg.h> +#include <stdbool.h> #include <stdio.h> #include <stdlib.h> #include <unistd.h> @@ -28,20 +31,51 @@ do \ { \ if (!(expr)) \ - { \ - fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ - fflush (stderr); \ - abort (); \ - } \ + { \ + fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (stderr); \ + abort (); \ + } \ } \ while (0) +#define BASE "test-openat.t" + +#include "test-open.h" + +static int dfd = AT_FDCWD; + +/* Wrapper around openat to test open behavior. */ +static int +do_open (char const *name, int flags, ...) +{ + if (flags & O_CREAT) + { + mode_t mode = 0; + va_list arg; + va_start (arg, flags); + + /* We have to use PROMOTED_MODE_T instead of mode_t, otherwise GCC 4 + creates crashing code when 'mode_t' is smaller than 'int'. */ + mode = va_arg (arg, PROMOTED_MODE_T); + + va_end (arg); + return openat (dfd, name, flags, mode); + } + return openat (dfd, name, flags); +} + int main (void) { - /* FIXME - add more tests. For example, share /dev/null and - trailing slash tests with test-open, and do more checks for - proper fd handling. */ + int result; + + /* Basic checks. */ + result = test_open (do_open, false); + dfd = open (".", O_RDONLY); + ASSERT (0 <= dfd); + ASSERT (test_open (do_open, false) == result); + ASSERT (close (dfd) == 0); /* Check that even when *-safer modules are in use, plain openat can land in fd 0. Do this test last, since it is destructive to @@ -49,12 +83,12 @@ ASSERT (close (STDIN_FILENO) == 0); ASSERT (openat (AT_FDCWD, ".", O_RDONLY) == STDIN_FILENO); { - int dfd = open (".", O_RDONLY); + dfd = open (".", O_RDONLY); ASSERT (STDIN_FILENO < dfd); ASSERT (chdir ("..") == 0); ASSERT (close (STDIN_FILENO) == 0); ASSERT (openat (dfd, ".", O_RDONLY) == STDIN_FILENO); ASSERT (close (dfd) == 0); } - return 0; + return result; }