From b4caad347f49b6fc8ec0b656e494dcd90f031b7c Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Sep 2009 15:55:24 -0600 Subject: [PATCH] lstat: fix Solaris 9 bug lstat("file/",buf) mistakenly succeeded. * lib/lstat.c (lstat): Also check for trailing slash on non-symlink, non-directories. Use stat module to simplify logic. * doc/posix-functions/lstat.texi (lstat): Document it. * modules/lstat-tests (Depends-on): Add errno, same-inode. (configure.ac): Check for symlink. * tests/test-lstat.c (main): Add more tests. Signed-off-by: Eric Blake --- ChangeLog | 8 +++ doc/posix-functions/lstat.texi | 17 +++++- lib/lstat.c | 41 +++++++------- modules/lstat-tests | 3 ++ tests/test-lstat.c | 119 ++++++++++++++++++++++++++++++++++++++--- 5 files changed, 159 insertions(+), 29 deletions(-) diff --git a/ChangeLog b/ChangeLog index 100272237..70c18c10e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,13 @@ 2009-09-19 Eric Blake + lstat: fix Solaris 9 bug + * lib/lstat.c (lstat): Also check for trailing slash on + non-symlink, non-directories. Use stat module to simplify logic. + * doc/posix-functions/lstat.texi (lstat): Document it. + * modules/lstat-tests (Depends-on): Add errno, same-inode. + (configure.ac): Check for symlink. + * tests/test-lstat.c (main): Add more tests. + stat: add as dependency to other modules * modules/chown (Depends-on): Add stat. * modules/euidaccess (Depends-on): Likewise. diff --git a/doc/posix-functions/lstat.texi b/doc/posix-functions/lstat.texi index 7a82d87bf..dbe31f863 100644 --- a/doc/posix-functions/lstat.texi +++ b/doc/posix-functions/lstat.texi @@ -9,8 +9,13 @@ Gnulib module: lstat Portability problems fixed by Gnulib: @itemize @item -When the argument ends in a slash, some platforms don't dereference the -argument. +For symlinks, when the argument ends in a slash, some platforms don't +dereference the argument: +Solaris 9. +@item +On some platforms, @code{lstat("file/",buf)} succeeds instead of +failing with @code{ENOTDIR}. +Solaris 9. @item On Windows platforms (excluding Cygwin), symlinks are not supported, so @code{lstat} does not exist. @@ -22,4 +27,12 @@ Portability problems not fixed by Gnulib: On platforms where @code{off_t} is a 32-bit type, @code{lstat} may not correctly report the size of files or block devices larger than 2 GB. The fix is to use the @code{AC_SYS_LARGEFILE} macro. +@item +On Windows platforms (excluding Cygwin), @code{st_ino} is always 0. +@item +Because of the definition of @code{struct stat}, it is not possible to +portably replace @code{stat} via an object-like macro. Therefore, +expressions such as @code{(islnk ? lstat : stat) (name, buf)} are not +portable, and should instead be written @code{islnk ? lstat (name, +buf) : stat (name, buf)}. @end itemize diff --git a/lib/lstat.c b/lib/lstat.c index 3e0727080..a05f6747d 100644 --- a/lib/lstat.c +++ b/lib/lstat.c @@ -1,6 +1,7 @@ /* Work around a bug of lstat on some systems - Copyright (C) 1997-1999, 2000-2006, 2008 Free Software Foundation, Inc. + Copyright (C) 1997-1999, 2000-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 @@ -19,7 +20,7 @@ #include -/* Get the original definition of open. It might be defined as a macro. */ +/* Get the original definition of lstat. It might be defined as a macro. */ #define __need_system_sys_stat_h #include #include @@ -56,27 +57,27 @@ rpl_lstat (const char *file, struct stat *sbuf) size_t len; int lstat_result = orig_lstat (file, sbuf); - if (lstat_result != 0 || !S_ISLNK (sbuf->st_mode)) + if (lstat_result != 0) return lstat_result; + /* This replacement file can blindly check against '/' rather than + using the ISSLASH macro, because all platforms with '\\' either + lack symlinks (mingw) or have working lstat (cygwin) and thus do + not compile this file. 0 len should have already been filtered + out above, with a failure return of ENOENT. */ len = strlen (file); - if (len == 0 || file[len - 1] != '/') + if (file[len - 1] != '/' || S_ISDIR (sbuf->st_mode)) return 0; - /* FILE refers to a symbolic link and the name ends with a slash. - Call stat() to get info about the link's referent. */ - - /* If stat fails, then we do the same. */ - if (stat (file, sbuf) != 0) - return -1; - - /* If FILE references a directory, return 0. */ - if (S_ISDIR (sbuf->st_mode)) - return 0; - - /* Here, we know stat succeeded and FILE references a non-directory. - But it was specified via a name including a trailing slash. - Fail with errno set to ENOTDIR to indicate the contradiction. */ - errno = ENOTDIR; - return -1; + /* At this point, a trailing slash is only permitted on + symlink-to-dir; but it should have found information on the + directory, not the symlink. Call stat() to get info about the + link's referent. Our replacement stat guarantees valid results, + even if the symlink is not pointing to a directory. */ + if (!S_ISLNK (sbuf->st_mode)) + { + errno = ENOTDIR; + return -1; + } + return stat (file, sbuf); } diff --git a/modules/lstat-tests b/modules/lstat-tests index f0fe3f65f..23686924a 100644 --- a/modules/lstat-tests +++ b/modules/lstat-tests @@ -2,8 +2,11 @@ Files: tests/test-lstat.c Depends-on: +errno +same-inode configure.ac: +AC_CHECK_FUNCS_ONCE([symlink]) Makefile.am: TESTS += test-lstat diff --git a/tests/test-lstat.c b/tests/test-lstat.c index 1a0a0f232..8182738a8 100644 --- a/tests/test-lstat.c +++ b/tests/test-lstat.c @@ -1,5 +1,5 @@ /* Test of lstat() function. - Copyright (C) 2008 Free Software Foundation, Inc. + Copyright (C) 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 @@ -14,24 +14,129 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ -/* Written by Simon Josefsson, 2008. */ +/* Written by Simon Josefsson, 2008; and Eric Blake, 2009. */ #include #include +#include +#include #include +#include +#include + +#include "same-inode.h" + +#if !HAVE_SYMLINK +# define symlink(a,b) (-1) +#endif + +#define ASSERT(expr) \ + do \ + { \ + if (!(expr)) \ + { \ + fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (stderr); \ + abort (); \ + } \ + } \ + while (0) + +#define BASE "test-lstat.t" int -main (int argc, char **argv) +main () { - struct stat buf; + struct stat st1; + struct stat st2; + + /* Remove any leftovers from a previous partial run. */ + ASSERT (system ("rm -rf " BASE "*") == 0); + + /* Test for common directories. */ + ASSERT (lstat (".", &st1) == 0); + ASSERT (lstat ("./", &st2) == 0); + ASSERT (SAME_INODE (st1, st2)); + ASSERT (S_ISDIR (st1.st_mode)); + ASSERT (S_ISDIR (st2.st_mode)); + ASSERT (lstat ("/", &st1) == 0); + ASSERT (lstat ("///", &st2) == 0); + ASSERT (SAME_INODE (st1, st2)); + ASSERT (S_ISDIR (st1.st_mode)); + ASSERT (S_ISDIR (st2.st_mode)); + ASSERT (lstat ("..", &st1) == 0); + ASSERT (S_ISDIR (st1.st_mode)); + + /* Test for error conditions. */ + errno = 0; + ASSERT (lstat ("", &st1) == -1); + ASSERT (errno == ENOENT); + errno = 0; + ASSERT (lstat ("nosuch", &st1) == -1); + ASSERT (errno == ENOENT); + errno = 0; + ASSERT (lstat ("nosuch/", &st1) == -1); + ASSERT (errno == ENOENT); - if (lstat ("/", &buf) != 0) + ASSERT (close (creat (BASE "file", 0600)) == 0); + ASSERT (lstat (BASE "file", &st1) == 0); + ASSERT (S_ISREG (st1.st_mode)); + errno = 0; + ASSERT (lstat (BASE "file/", &st1) == -1); + ASSERT (errno == ENOTDIR); + + /* Now for some symlink tests, where supported. We set up: + link1 -> directory + link2 -> file + link3 -> dangling + link4 -> loop + then test behavior both with and without trailing slash. + */ + if (symlink (".", BASE "link1") != 0) { - perror ("lstat"); - return 1; + ASSERT (unlink (BASE "file") == 0); + fputs ("skipping test: symlinks not supported on this filesystem\n", + stderr); + return 77; } + ASSERT (symlink (BASE "file", BASE "link2") == 0); + ASSERT (symlink (BASE "nosuch", BASE "link3") == 0); + ASSERT (symlink (BASE "link4", BASE "link4") == 0); + + ASSERT (lstat (BASE "link1", &st1) == 0); + ASSERT (S_ISLNK (st1.st_mode)); + ASSERT (lstat (BASE "link1/", &st1) == 0); + ASSERT (stat (BASE "link1", &st2) == 0); + ASSERT (S_ISDIR (st1.st_mode)); + ASSERT (S_ISDIR (st2.st_mode)); + ASSERT (SAME_INODE (st1, st2)); + + ASSERT (lstat (BASE "link2", &st1) == 0); + ASSERT (S_ISLNK (st1.st_mode)); + errno = 0; + ASSERT (lstat (BASE "link2/", &st1) == -1); + ASSERT (errno == ENOTDIR); + + ASSERT (lstat (BASE "link3", &st1) == 0); + ASSERT (S_ISLNK (st1.st_mode)); + errno = 0; + ASSERT (lstat (BASE "link3/", &st1) == -1); + ASSERT (errno == ENOENT); + + ASSERT (lstat (BASE "link4", &st1) == 0); + ASSERT (S_ISLNK (st1.st_mode)); + errno = 0; + ASSERT (lstat (BASE "link4/", &st1) == -1); + ASSERT (errno == ELOOP); + + /* Cleanup. */ + ASSERT (unlink (BASE "file") == 0); + ASSERT (unlink (BASE "link1") == 0); + ASSERT (unlink (BASE "link2") == 0); + ASSERT (unlink (BASE "link3") == 0); + ASSERT (unlink (BASE "link4") == 0); return 0; } -- 2.11.0