Re: improve unlink on Solaris

[Eric Blake]

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

According to Eric Blake on 9/19/2009 2:04 PM:
> Here's the polished series; now pushed.  I've tested on Linux, cygwin 1.5,
> cygwin 1.7, mingw, OpenBSD, Solaris 8, 9, 10.
> 
>       [6/12] remove: new module, for mingw and Solaris 9 bugs
>       [7/12] test-fstatat: new test, to expose Solaris 9 bugs
>       [8/12] test-unlinkat: enhance test, to expose Solaris 9 bug

Theoretically, these tests could create an unintentional directory hard
link if run as root, so I've tightened that up a bit (although I don't
have root access to a Solaris machine to test).  Also, I'm tweaking the
tests to ensure we don't repeat a bug present in current cygwin sources,
where "a/./" is treated differently than "a/.//" (the unlinkat test now
fails on cygwin 1.7, but I've posted a patch to the cygwin list as well,
so that gnulib doesn't have to worry about it).  Pushing this:

- --
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/

iEYEARECAAYFAkrElaoACgkQ84KuGfSFAYDz8gCgpYRxAByXOsS5IlWR/MnFVpfF
MUwAmQF0z5n8xKQBOpKWOcQXHyjoYNAW
=TMSO
-----END PGP SIGNATURE-----

>From 0f4fd7f4782907e0a308a077118c6578fff4e49f Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 29 Sep 2009 11:40:49 -0600
Subject: [PATCH] tests: tighten link, rmdir, and remove tests

Catch cygwin bug where rmdir("a/./") failed but rmdir("a/.//")
succeeded.  Be kinder if a Solaris root user runs the test.

* tests/test-link.h (includes): No need to use <config.h> here.
Clean up if directory hard link was created, otherwise test for
trailing '.'.
* tests/test-linkat.c (main): Simplify.
* tests/test-remove.c (main): Enhance test for trailing '.'.
* tests/test-rmdir.h (test_rmdir_func): Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog           |   10 ++++++++++
 tests/test-link.h   |   39 ++++++++++++++++++++++++++++++---------
 tests/test-linkat.c |   40 ++++++++++++++++++----------------------
 tests/test-remove.c |    2 +-
 tests/test-rmdir.h  |    2 +-
 5 files changed, 60 insertions(+), 33 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3cecf35..7ad1595 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2009-10-01  Eric Blake  <address@hidden>
+
+       tests: tighten link, rmdir, and remove tests
+       * tests/test-link.h (includes): No need to use <config.h> here.
+       Clean up if directory hard link was created, otherwise test for
+       trailing '.'.
+       * tests/test-linkat.c (main): Simplify.
+       * tests/test-remove.c (main): Enhance test for trailing '.'.
+       * tests/test-rmdir.h (test_rmdir_func): Likewise.
+
 2009-10-01  Jim Meyering  <address@hidden>

        maint.mk: requiring "make major" was annoying, for a "minor" release.
diff --git a/tests/test-link.h b/tests/test-link.h
index 9ce1894..4d08e7b 100644
--- a/tests/test-link.h
+++ b/tests/test-link.h
@@ -14,13 +14,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

-#include <config.h>
-
 /* This file is designed to test both link(a,b) and
-   linkat(AT_FDCWD,a,AT_FDCWD,b).  FUNC is the function to test.
+   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 symlink tests with status 77.  */
+   skipping tests with status 77.  This test does not exercise link on
+   symlinks.  */

 static int
 test_link (int (*func) (char const *, char const *), bool print)
@@ -93,8 +92,7 @@ test_link (int (*func) (char const *, char const *), bool 
print)
     ASSERT (close (fd) == 0);
   }

-  /* Test for various error conditions.  Assumes hard links to
-     directories are not permitted.  */
+  /* Test for various error conditions.  */
   ASSERT (mkdir (BASE "d", 0700) == 0);
   errno = 0;
   ASSERT (func (BASE "a", ".") == -1);
@@ -121,9 +119,32 @@ test_link (int (*func) (char const *, char const *), bool 
print)
   errno = 0;
   ASSERT (func (BASE "a", BASE "c/") == -1);
   ASSERT (errno == ENOTDIR || errno == ENOENT);
-  errno = 0;
-  ASSERT (func (BASE "d", BASE "c") == -1);
-  ASSERT (errno == EPERM || errno == EACCES);
+
+  /* Most platforms reject hard links to directories, and even on
+     those that do permit it, most users can't create them.  We assume
+     that if this test is run as root and we managed to create a hard
+     link, then unlink better be able to clean it up.  */
+  {
+    int result;
+    errno = 0;
+    result = func (BASE "d", BASE "c");
+    if (result == 0)
+      {
+        /* Probably root on Solaris.  */
+        ASSERT (unlink (BASE "c") == 0);
+      }
+    else
+      {
+        /* Most everyone else.  */
+        ASSERT (errno == EPERM || errno == EACCES);
+        errno = 0;
+        ASSERT (func (BASE "d/.", BASE "c") == -1);
+        ASSERT (errno == EPERM || errno == EACCES || errno == EINVAL);
+        errno = 0;
+        ASSERT (func (BASE "d/.//", BASE "c") == -1);
+        ASSERT (errno == EPERM || errno == EACCES || errno == EINVAL);
+      }
+  }

   /* Clean up.  */
   ASSERT (unlink (BASE "a") == 0);
diff --git a/tests/test-linkat.c b/tests/test-linkat.c
index f5c4c77..6aa3ca7 100644
--- a/tests/test-linkat.c
+++ b/tests/test-linkat.c
@@ -92,32 +92,28 @@ main ()
   ASSERT (system ("rm -rf " BASE "*") == 0);

   /* Test basic link functionality, without mentioning symlinks.  */
-  {
-    result = test_link (do_link, false);
-    dfd1 = open (".", O_RDONLY);
-    ASSERT (0 <= dfd1);
-    ASSERT (test_link (do_link, false) == result);
-    dfd2 = dfd1;
-    ASSERT (test_link (do_link, false) == result);
-    dfd1 = AT_FDCWD;
-    ASSERT (test_link (do_link, false) == result);
-    flag = 0;
-    ASSERT (test_link (do_link, false) == result);
-    dfd1 = dfd2;
-    ASSERT (test_link (do_link, false) == result);
-    dfd2 = AT_FDCWD;
-    ASSERT (test_link (do_link, false) == result);
-    ASSERT (close (dfd1) == 0);
-    dfd1 = AT_FDCWD;
-    ASSERT (test_link (do_link, false) == result);
-  }
+  result = test_link (do_link, false);
+  dfd1 = open (".", O_RDONLY);
+  ASSERT (0 <= dfd1);
+  ASSERT (test_link (do_link, false) == result);
+  dfd2 = dfd1;
+  ASSERT (test_link (do_link, false) == result);
+  dfd1 = AT_FDCWD;
+  ASSERT (test_link (do_link, false) == result);
+  flag = 0;
+  ASSERT (test_link (do_link, false) == result);
+  dfd1 = dfd2;
+  ASSERT (test_link (do_link, false) == result);
+  dfd2 = AT_FDCWD;
+  ASSERT (test_link (do_link, false) == result);
+  ASSERT (close (dfd1) == 0);
+  dfd1 = AT_FDCWD;
+  ASSERT (test_link (do_link, false) == result);

   /* Create locations to manipulate.  */
   ASSERT (mkdir (BASE "sub1", 0700) == 0);
   ASSERT (mkdir (BASE "sub2", 0700) == 0);
-  dfd = creat (BASE "00", 0600);
-  ASSERT (0 <= dfd);
-  ASSERT (close (dfd) == 0);
+  ASSERT (close (creat (BASE "00", 0600)) == 0);
   cwd = xgetcwd ();

   dfd = open (BASE "sub1", O_RDONLY);
diff --git a/tests/test-remove.c b/tests/test-remove.c
index 99edb0c..c084d8c 100644
--- a/tests/test-remove.c
+++ b/tests/test-remove.c
@@ -83,7 +83,7 @@ main ()

   /* Empty directory.  */
   errno = 0;
-  ASSERT (remove (BASE "dir/./") == -1);
+  ASSERT (remove (BASE "dir/.//") == -1);
   ASSERT (errno == EINVAL || errno == EBUSY);
   ASSERT (remove (BASE "dir") == 0);

diff --git a/tests/test-rmdir.h b/tests/test-rmdir.h
index bb7b344..94636dd 100644
--- a/tests/test-rmdir.h
+++ b/tests/test-rmdir.h
@@ -68,7 +68,7 @@ test_rmdir_func (int (*func) (char const *name), bool print)
   /* Empty directory.  */
   ASSERT (unlink (BASE "dir/file") == 0);
   errno = 0;
-  ASSERT (func (BASE "dir/./") == -1);
+  ASSERT (func (BASE "dir/.//") == -1);
   ASSERT (errno == EINVAL || errno == EBUSY);
   ASSERT (func (BASE "dir") == 0);

-- 
1.6.5.rc1