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 <ebb9@byu.net>

5 files changed, 159 insertions, 29 deletions

diff --git a/ChangeLog b/ChangeLog
index 100272237f..70c18c10ef 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-09-19  Eric Blake  <ebb9@byu.net>
 
+	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 7a82d87bfe..dbe31f8636 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 3e07270801..a05f6747d3 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 <config.h>
 
-/* 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 <sys/types.h>
 #include <sys/stat.h>
@@ -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 f0fe3f65f9..23686924a7 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 1a0a0f232a..8182738a86 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 <http://www.gnu.org/licenses/>.  */
 
-/* Written by Simon Josefsson, 2008.  */
+/* Written by Simon Josefsson, 2008; and Eric Blake, 2009.  */
 
 #include <config.h>
 
 #include <sys/stat.h>
 
+#include <fcntl.h>
+#include <errno.h>
 #include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#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;
 }