Re: 4 test failures on freebsd8-rc2

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 11/2/2009 8:37 PM:
> According to Jim Meyering on 11/2/2009 2:18 PM:
>> test-linkat.c:240: assertion failed
> ...
>> test-utimens.h:126: assertion failed
> 
> These two failures look similar, in that they are both instances of the
> BSD kernel mistakenly succeeding when handed a symlink to a non-directory
> coupled with a trailing slash.  It's not quite the same failure scenario
> as Solaris 9, but the workaround is similar.  So I'm hoping that all I
> have to do is update the .m4 tests to detect the bug, and that the
> existing Solaris 9 code will take care of the rest.

Here's part of the difference - Solaris 9 mistakenly allows stat("file/")
and stat("link-to-file/") to succeed, while FreeBSD only allowed the
latter. Since the m4 test only covered the former, we weren't replacing
stat on FreeBSD, even though we needed to.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkr2DOwACgkQ84KuGfSFAYAUPQCdFpoXVvwzfnzk/ULwLnCwAkIh
84MAniwEFbaYkoQeuzuxgOw5VuLI5tZh
=ZRbK
-----END PGP SIGNATURE-----

>From 80b6bb689dbadffa4636fc958b9e0e87a57d2834 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Sat, 7 Nov 2009 16:59:11 -0700
Subject: [PATCH] stat: detect FreeBSD bug

Like Solaris 9, FreeBSD 7.2 mistakenly allows stat("link-to-file/").
Unlike Solaris, it correctly forbids stat("file/").  A number of
interfaces are affected (such as utimes), but replacing stat is
enough to catch the remainder by reusing the Solaris 9 fixes.

* m4/stat.m4 (gl_FUNC_STAT): Reject FreeBSD stat.
* doc/posix-functions/stat.texi (stat): Document the bug.
* tests/test-stat.h (test_stat_func): Add argument.
* tests/test-stat.c (main): Adjust caller.
* tests/test-fstatat.c (main): Likewise.
* modules/stat-tests (Depends-on): Add stdbool, symlink.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                     |   10 +++++++++
 doc/posix-functions/stat.texi |    6 ++--
 m4/stat.m4                    |   18 +++++++++++++--
 modules/stat-tests            |    2 +
 tests/test-fstatat.c          |   18 ++++++++--------
 tests/test-stat.c             |   13 ++++++-----
 tests/test-stat.h             |   45 +++++++++++++++++++++++++++++++++++++++-
 7 files changed, 89 insertions(+), 23 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c9d05ad..dfdf8d1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2009-11-07  Eric Blake  <address@hidden>

+       stat: detect FreeBSD bug
+       * m4/stat.m4 (gl_FUNC_STAT): Reject FreeBSD stat.
+       * doc/posix-functions/stat.texi (stat): Document the bug.
+       * tests/test-stat.h (test_stat_func): Add argument.
+       * tests/test-stat.c (main): Adjust caller.
+       * tests/test-fstatat.c (main): Likewise.
+       * modules/stat-tests (Depends-on): Add stdbool, symlink.
+
+2009-11-07  Eric Blake  <address@hidden>
+
        fflush, freadseek: use fseeko, not fseek
        * lib/fflush.c (clear_ungetc_buffer_preserving_position)
        (clear_ungetc_buffer): Avoid potential problems on large files.
diff --git a/doc/posix-functions/stat.texi b/doc/posix-functions/stat.texi
index 5fdb683..ef09740 100644
--- a/doc/posix-functions/stat.texi
+++ b/doc/posix-functions/stat.texi
@@ -9,9 +9,9 @@ stat
 Portability problems fixed by Gnulib:
 @itemize
 @item
-On some platforms, @code{stat("file/",buf)} succeeds instead of
-failing with @code{ENOTDIR}.
-Solaris 9.
+On some platforms, @code{stat("link-to-file/",buf)} succeeds instead
+of failing with @code{ENOTDIR}.
+FreeBSD 7.2, Solaris 9.
 @item
 On some platforms, @code{stat(".",buf)} and @code{stat("./",buf)} give
 different results:
diff --git a/m4/stat.m4 b/m4/stat.m4
index 0a10de1..ce6933b 100644
--- a/m4/stat.m4
+++ b/m4/stat.m4
@@ -1,4 +1,4 @@
-# serial 2
+# serial 3

 # Copyright (C) 2009 Free Software Foundation, Inc.
 #
@@ -11,6 +11,7 @@ AC_DEFUN([gl_FUNC_STAT],
   AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
   AC_REQUIRE([gl_AC_DOS])
   AC_REQUIRE([gl_SYS_STAT_H_DEFAULTS])
+  AC_CHECK_FUNCS_ONCE([lstat])
   dnl mingw is the only known platform where stat(".") and stat("./") differ
   AC_CACHE_CHECK([whether stat handles trailing slashes on directories],
       [gl_cv_func_stat_dir_slash],
@@ -24,15 +25,26 @@ AC_DEFUN([gl_FUNC_STAT],
             *) gl_cv_func_stat_dir_slash="guessing yes";;
           esac])])
   dnl Solaris 9 mistakenly succeeds on stat("file/")
+  dnl FreeBSD 7.2 mistakenly succeeds on stat("link-to-file/")
   AC_CACHE_CHECK([whether stat handles trailing slashes on files],
       [gl_cv_func_stat_file_slash],
       [touch conftest.tmp
+       # Assume that if we have lstat, we can also check symlinks.
+       if test $ac_cv_func_lstat = yes; then
+         ln -s conftest.tmp conftest.lnk
+       fi
        AC_RUN_IFELSE(
          [AC_LANG_PROGRAM(
            [[#include <sys/stat.h>
-]], [[struct stat st; return !stat ("conftest.tmp/", &st);]])],
+]], [[struct stat st;
+      if (!stat ("conftest.tmp/", &st)) return 1;
+#if HAVE_LSTAT
+      if (!stat ("conftest.lnk/", &st)) return 2;
+#endif
+           ]])],
          [gl_cv_func_stat_file_slash=yes], [gl_cv_func_stat_file_slash=no],
-         [gl_cv_func_stat_file_slash="guessing no"])])
+         [gl_cv_func_stat_file_slash="guessing no"])
+       rm -f conftest.tmp conftest.lnk])
   case $gl_cv_func_stat_dir_slash in
     *no) REPLACE_STAT=1
       AC_DEFINE([REPLACE_FUNC_STAT_DIR], [1], [Define to 1 if stat needs
diff --git a/modules/stat-tests b/modules/stat-tests
index 93444aa..eb81446 100644
--- a/modules/stat-tests
+++ b/modules/stat-tests
@@ -5,6 +5,8 @@ tests/test-stat.c
 Depends-on:
 pathmax
 same-inode
+stdbool
+symlink

 configure.ac:

diff --git a/tests/test-fstatat.c b/tests/test-fstatat.c
index ad5434e..e965997 100644
--- a/tests/test-fstatat.c
+++ b/tests/test-fstatat.c
@@ -35,11 +35,11 @@
   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)

@@ -68,11 +68,11 @@ int
 main (void)
 {
   int result;
-  ASSERT (test_stat_func (do_stat) == 0);
-  result = test_lstat_func (do_lstat, false);
+  result = test_stat_func (do_stat, false);
+  ASSERT (test_lstat_func (do_lstat, false) == result);
   dfd = open (".", O_RDONLY);
   ASSERT (0 <= dfd);
-  ASSERT (test_stat_func (do_stat) == 0);
+  ASSERT (test_stat_func (do_stat, false) == result);
   ASSERT (test_lstat_func (do_lstat, false) == result);
   ASSERT (close (dfd) == 0);

@@ -80,6 +80,6 @@ main (void)

   if (result == 77)
     fputs ("skipping test: symlinks not supported on this file system\n",
-          stderr);
+           stderr);
   return result;
 }
diff --git a/tests/test-stat.c b/tests/test-stat.c
index 61a9d90..fc11731 100644
--- a/tests/test-stat.c
+++ b/tests/test-stat.c
@@ -22,6 +22,7 @@

 #include <fcntl.h>
 #include <errno.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -33,11 +34,11 @@
   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)

@@ -56,5 +57,5 @@ do_stat (char const *name, struct stat *st)
 int
 main (void)
 {
-  return test_stat_func (do_stat);
+  return test_stat_func (do_stat, true);
 }
diff --git a/tests/test-stat.h b/tests/test-stat.h
index 17bb43f..8f7897c 100644
--- a/tests/test-stat.h
+++ b/tests/test-stat.h
@@ -19,10 +19,11 @@
 /* This file is designed to test both stat(n,buf) and
    fstatat(AT_FDCWD,n,buf,0).  FUNC is the function to test.  Assumes
    that BASE and ASSERT are already defined, and that appropriate
-   headers are already included.  */
+   headers are already included.  If PRINT, warn before skipping
+   symlink tests with status 77.  */

 static int
-test_stat_func (int (*func) (char const *, struct stat *))
+test_stat_func (int (*func) (char const *, struct stat *), bool print)
 {
   struct stat st1;
   struct stat st2;
@@ -53,7 +54,47 @@ test_stat_func (int (*func) (char const *, struct stat *))
   errno = 0;
   ASSERT (func (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 with trailing slash.
+  */
+  if (symlink (".", BASE "link1") != 0)
+    {
+      ASSERT (unlink (BASE "file") == 0);
+      if (print)
+        fputs ("skipping test: symlinks not supported on this file system\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 (func (BASE "link1/", &st1) == 0);
+  ASSERT (S_ISDIR (st1.st_mode));
+
+  errno = 0;
+  ASSERT (func (BASE "link2/", &st1) == -1);
+  ASSERT (errno == ENOTDIR);
+
+  errno = 0;
+  ASSERT (func (BASE "link3/", &st1) == -1);
+  ASSERT (errno == ENOENT);
+
+  errno = 0;
+  ASSERT (func (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;
 }
-- 
1.6.5.rc1