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