Re: 4 test failures on freebsd8-rc2

[Eric Blake]

Jim Meyering <jim <at> meyering.net> writes:

> If I'd known this was coming so quickly, I would have delayed
> making the coreutils snapshot.
> 
> Let me know when you're done: maybe I'll make one more
> before the release.

At least a couple more to go.  I've confirmed that on FreeBSD:

touch f
ln -s f l
link l/ g
unlink l/

creates g as a hard link to f, then removes f; both the link and the unlink 
should have failed, since l/ is not a symlink-to-directory.  Hmm, that means 
coreutils' testsuite could use more trailing slash tests, so I'll add that to 
my todo list.  Here's what I'm currently testing on the gnulib side...

I also suspect problems in chmod and chown, but have not had time to confirm 
that yet.

>From 120a86a14b534bf24808974967786611f16dc8c0 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 9 Nov 2009 10:44:08 -0700
Subject: [PATCH 1/2] unlink, remove: detect FreeBSD bug

unlink("link-to-file/") mistakenly removed "file".

* m4/unlink.m4 (gl_FUNC_UNLINK): Also detect FreeBSD bug with
slash on symlink.
* doc/posix-functions/unlink.texi (unlink): Document the bug.
* doc/posix-functions/remove.texi (remove): Likewise.
* tests/test-unlink.h (test_unlink): Enhance test.
* tests/test-remove.c (main): Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                       |   10 ++++++++++
 doc/posix-functions/remove.texi |    2 +-
 doc/posix-functions/unlink.texi |    4 ++--
 m4/unlink.m4                    |   20 ++++++++++++++------
 tests/test-remove.c             |   13 +++++++++++++
 tests/test-unlink.h             |    7 +++++--
 6 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8ee7c8e..f451319 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2009-11-09  Eric Blake  <address@hidden>
+
+       unlink, remove: detect FreeBSD bug
+       * m4/unlink.m4 (gl_FUNC_UNLINK): Also detect FreeBSD bug with
+       slash on symlink.
+       * doc/posix-functions/unlink.texi (unlink): Document the bug.
+       * doc/posix-functions/remove.texi (remove): Likewise.
+       * tests/test-unlink.h (test_unlink): Enhance test.
+       * tests/test-remove.c (main): Likewise.
+
 2009-10-30  Eric Blake  <address@hidden>

        vasnprintf: avoid compiler warnings
diff --git a/doc/posix-functions/remove.texi b/doc/posix-functions/remove.texi
index b3e3d1b..83a8955 100644
--- a/doc/posix-functions/remove.texi
+++ b/doc/posix-functions/remove.texi
@@ -11,7 +11,7 @@ remove
 @item
 This function fails to reject trailing slashes on non-directories on
 some platforms:
-Solaris 9.
+FreeBSD 7.2, Solaris 9.
 @item
 This function mistakenly removes a directory with
 @code{remove("dir/./")} on some platforms:
diff --git a/doc/posix-functions/unlink.texi b/doc/posix-functions/unlink.texi
index 1df1e30..42b9e81 100644
--- a/doc/posix-functions/unlink.texi
+++ b/doc/posix-functions/unlink.texi
@@ -9,8 +9,8 @@ unlink
 Portability problems fixed by Gnulib:
 @itemize
 @item
-Some systems mistakenly succeed on @code{unlink("file/")}:
-GNU/Hurd, Solaris 9.
+Some systems mistakenly succeed on @code{unlink("link-to-file/")}:
+GNU/Hurd, FreeBSD 7.2, Solaris 9.
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/m4/unlink.m4 b/m4/unlink.m4
index 626c3ad..376e584 100644
--- a/m4/unlink.m4
+++ b/m4/unlink.m4
@@ -1,4 +1,4 @@
-# unlink.m4 serial 1
+# unlink.m4 serial 2
 dnl Copyright (C) 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,
@@ -8,18 +8,26 @@ AC_DEFUN([gl_FUNC_UNLINK],
 [
   AC_REQUIRE([gl_AC_DOS])
   AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
-  dnl Detect Solaris 9 bug.
+  dnl Detect Solaris 9 and FreeBSD 7.2 bug.
   AC_CACHE_CHECK([whether unlink honors trailing slashes],
     [gl_cv_func_unlink_works],
     [touch conftest.file
+     # Assume that if we have lstat, we can also check symlinks.
+     if test $ac_cv_func_lstat = yes; then
+       ln -s conftest.file conftest.lnk
+     fi
      AC_RUN_IFELSE(
        [AC_LANG_PROGRAM(
-         [[#include <stdio.h>
-           #include <errno.h>
-]], [[return !unlink ("conftest.file/") || errno != ENOTDIR;]])],
+        [[#include <stdio.h>
+          #include <errno.h>
+]], [[if (!unlink ("conftest.file/") || errno != ENOTDIR;) return 1;
+#if HAVE_LSTAT
+      if (!unlink ("conftest.lnk/") || errno != ENOTDIR;) return 2;
+#endif
+      ]])],
       [gl_cv_func_unlink_works=yes], [gl_cv_func_unlink_works=no],
       [gl_cv_func_unlink_works="guessing no"])
-     rm -f conftest.file])
+     rm -f conftest.file conftest.lnk])
   if test x"$gl_cv_func_unlink_works" != xyes; then
     REPLACE_UNLINK=1
     AC_LIBOBJ([unlink])
diff --git a/tests/test-remove.c b/tests/test-remove.c
index af5d01b..16bfe2f 100644
--- a/tests/test-remove.c
+++ b/tests/test-remove.c
@@ -113,6 +113,19 @@ main (void)
     ASSERT (S_ISLNK (st.st_mode));
   }
   ASSERT (remove (BASE "link") == 0);
+  /* Trailing slash on symlink to non-directory is an error.  */
+  ASSERT (symlink (BASE "loop", BASE "loop") == 0);
+  errno = 0;
+  ASSERT (remove (BASE "loop/") == -1);
+  ASSERT (errno == ELOOP || errno == ENOTDIR);
+  ASSERT (remove (BASE "loop") == 0);
+  ASSERT (close (creat (BASE "file", 0600)) == 0);
+  ASSERT (symlink (BASE "file", BASE "link") == 0);
+  errno = 0;
+  ASSERT (remove (BASE "link/") == -1);
+  ASSERT (errno == ENOTDIR);
+  ASSERT (remove (BASE "link") == 0);
+  ASSERT (remove (BASE "file") == 0);

   return 0;
 }
diff --git a/tests/test-unlink.h b/tests/test-unlink.h
index 6b6384e..e65057d 100644
--- a/tests/test-unlink.h
+++ b/tests/test-unlink.h
@@ -68,14 +68,17 @@ test_unlink_func (int (*func) (char const *name), bool 
print)
       ASSERT (func (BASE "dir/file") == 0);
       ASSERT (rmdir (BASE "dir") == 0);
       if (print)
-       fputs ("skipping test: symlinks not supported on this file system\n",
-              stderr);
+        fputs ("skipping test: symlinks not supported on this file system\n",
+               stderr);
       return 77;
     }
   if (cannot_unlink_dir ())
     ASSERT (func (BASE "link/") == -1);
   ASSERT (func (BASE "link") == 0);
   ASSERT (symlink (BASE "dir/file", BASE "link") == 0);
+  errno = 0;
+  ASSERT (func (BASE "link/") == -1);
+  ASSERT (errno == ENOTDIR);
   /* Order here proves unlink of a symlink does not follow through to
      the file.  */
   ASSERT (func (BASE "link") == 0);
-- 
1.6.4.2


>From e5dc9ed77a2b00d2fca93f7bdfbf4e1482e696c9 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 9 Nov 2009 14:23:11 -0700
Subject: [PATCH 2/2] link: detect FreeBSD bug

link("link-to-file/","a") mistakenly created "a" as a link to "file".

* m4/link.m4 (gl_FUNC_LINK): Also detect FreeBSD bug with slash on
symlink.
* doc/posix-functions/link.texi (link): Document the bug.
* tests/test-link.h (test_link): Enhance test.
* tests/test-linkat.c (main): Update caller.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                     |    7 +++++++
 doc/posix-functions/link.texi |    2 +-
 m4/link.m4                    |   14 +++++++++++---
 tests/test-link.h             |   28 +++++++++++++++++++++++-----
 tests/test-linkat.c           |   17 +++++++++--------
 5 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f451319..5b64e07 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2009-11-09  Eric Blake  <address@hidden>

+       link: detect FreeBSD bug
+       * m4/link.m4 (gl_FUNC_LINK): Also detect FreeBSD bug with slash on
+       symlink.
+       * doc/posix-functions/link.texi (link): Document the bug.
+       * tests/test-link.h (test_link): Enhance test.
+       * tests/test-linkat.c (main): Update caller.
+
        unlink, remove: detect FreeBSD bug
        * m4/unlink.m4 (gl_FUNC_UNLINK): Also detect FreeBSD bug with
        slash on symlink.
diff --git a/doc/posix-functions/link.texi b/doc/posix-functions/link.texi
index c06f0a6..6279fe8 100644
--- a/doc/posix-functions/link.texi
+++ b/doc/posix-functions/link.texi
@@ -14,7 +14,7 @@ link
 @item
 This function fails to reject trailing slashes on non-directories on
 some platforms:
-Solaris, Cygwin 1.5.x.
+FreeBSD 7.2, Solaris, Cygwin 1.5.x.
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/m4/link.m4 b/m4/link.m4
index a212ee2..d5fcef9 100644
--- a/m4/link.m4
+++ b/m4/link.m4
@@ -1,4 +1,4 @@
-# link.m4 serial 3
+# link.m4 serial 4
 dnl Copyright (C) 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,
@@ -15,13 +15,21 @@ AC_DEFUN([gl_FUNC_LINK],
     AC_CACHE_CHECK([whether link handles trailing slash correctly],
       [gl_cv_func_link_works],
       [touch conftest.a
+       # Assume that if we have lstat, we can also check symlinks.
+       if test $ac_cv_func_lstat = yes; then
+         ln -s conftest.a conftest.lnk
+       fi
        AC_RUN_IFELSE(
          [AC_LANG_PROGRAM(
            [[#include <unistd.h>
-]], [[return !link ("conftest.a", "conftest.b/");]])],
+]], [[if (!link ("conftest.a", "conftest.b/")) return 1;
+#if HAVE_LSTAT
+      if (!link ("conftest.lnk/", "conftest.b")) return 2;
+#endif
+           ]])],
          [gl_cv_func_link_works=yes], [gl_cv_func_link_works=no],
          [gl_cv_func_link_works="guessing no"])
-       rm -f conftest.a conftest.b])
+       rm -f conftest.a conftest.b conftest.lnk])
     if test "$gl_cv_func_link_works" != yes; then
       REPLACE_LINK=1
       AC_LIBOBJ([link])
diff --git a/tests/test-link.h b/tests/test-link.h
index 363097d..bd9ac73 100644
--- a/tests/test-link.h
+++ b/tests/test-link.h
@@ -18,8 +18,8 @@
    linkat(AT_FDCWD,a,AT_FDCWD,b,0).  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 tests with status 77.  This test does not exercise link on
-   symlinks.  */
+   skipping tests with status 77.  This test does not try to create
+   hard links to symlinks, but does test other aspects of symlink.  */

 static int
 test_link (int (*func) (char const *, char const *), bool print)
@@ -145,14 +145,32 @@ test_link (int (*func) (char const *, char const *), bool 
print)
         ASSERT (errno == EPERM || errno == EACCES || errno == EINVAL);
       }
   }
-
-  /* Clean up.  */
   ASSERT (unlink (BASE "a") == 0);
-  ASSERT (unlink (BASE "b") == 0);
   errno = 0;
   ASSERT (unlink (BASE "c") == -1);
   ASSERT (errno == ENOENT);
   ASSERT (rmdir (BASE "d") == 0);

+  /* Test invalid use of symlink.  */
+  if (symlink (BASE "a", BASE "link") != 0)
+    {
+      ASSERT (unlink (BASE "b") == 0);
+      if (print)
+        fputs ("skipping test: symlinks not supported on this file system\n",
+               stderr);
+      return 77;
+    }
+  errno = 0;
+  ASSERT (func (BASE "b", BASE "link/") == -1);
+  ASSERT (errno == ENOTDIR || errno == ENOENT);
+  ASSERT (rename (BASE "b", BASE "a") == 0);
+  errno = 0;
+  ASSERT (func (BASE "link/", BASE "b") == -1);
+  ASSERT (errno == ENOTDIR);
+
+  /* Clean up.  */
+  ASSERT (unlink (BASE "a") == 0);
+  ASSERT (unlink (BASE "link") == 0);
+
   return 0;
 }
diff --git a/tests/test-linkat.c b/tests/test-linkat.c
index b06b318..11233fe 100644
--- a/tests/test-linkat.c
+++ b/tests/test-linkat.c
@@ -36,11 +36,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)

@@ -92,7 +92,7 @@ main (void)
   ASSERT (system ("rm -rf " BASE "*") == 0);

   /* Test basic link functionality, without mentioning symlinks.  */
-  result = test_link (do_link, false);
+  result = test_link (do_link, true);
   dfd1 = open (".", O_RDONLY);
   ASSERT (0 <= dfd1);
   ASSERT (test_link (do_link, false) == result);
@@ -166,8 +166,9 @@ main (void)
       ASSERT (rmdir (BASE "sub1") == 0);
       ASSERT (rmdir (BASE "sub2") == 0);
       free (cwd);
-      fputs ("skipping test: symlinks not supported on this file system\n",
-             stderr);
+      if (!result)
+        fputs ("skipping test: symlinks not supported on this file system\n",
+               stderr);
       return result;
     }
   dfd = open (".", O_RDONLY);
-- 
1.6.4.2