Re: unlink test failure on MacOS X

[Bruno Haible]

Hi Eric,

> > This prints
> > -1 21 [EISDIR]
> > on Linux, but
> > 0 0
> > on MacOS X.
> 
> Which version of MacOS X?  What does uname state?  What file system?

For me, it happens on MacOS X 10.5.6. I could only test HFS+.

> I cannot reproduce it on the machine I have access to, tested on an nfs
> mount (where built) and hfs mount (/tmp):
> 
> $ uname -v
> Darwin Kernel Version 9.8.0: Wed Jul 15 16:55:01 PDT 2009;
> root:xnu-1228.15.4~1/RELEASE_I386
> $ ./foo
> -1 1
> $ cd /tmp
> $ ~/foo
> -1 16

Possible causes are:
  - You are in /tmp, trying to unlink the root directory, not a normal HFS
    directory.
  - Something has changed between MacOS X 10.5.6 and 10.5.8. In fact, I can
    see some changes in the area of deletion of a VFS node (between
    xnu-1228.9.59/bsd/vfs/vfs_subr.c and xnu-1228.15.4/bsd/vfs/vfs_subr.c).

> The fact that it may be file-system dependent makes the fix trickier -
> we now have to test hostname and assume that MacOS is broken, because it
> is no longer possible to assume that the fs used during ./configure is
> representative of other fs available on the system.

But we know that under /tmp, we are on a HFS file system (there's no way
to install MacOS X on a different file system, AFAIK).

Jim Meyering wrote:
> I wouldn't mind additional run-time code to detect that flaw
> and map it to sensible return and errno values,
> as long as it impacts only MacOS systems.

The workaround below impacts only MacOS platforms. I'm applying it.
It also fixes the 'unlinkat' test failure.


2010-03-20  Bruno Haible  <address@hidden>

        Work around unlink() bug on MacOS X 10.5.6.
        * lib/unlink.c (rpl_unlink): If UNLINK_PARENT_BUG is defined, fail when
        attempting to unlink a parent directory.
        * m4/unlink.m4 (gl_FUNC_UNLINK): Require AC_CANONICAL_HOST. Test for
        MacOS X 10.5 bug. If the bug is present, define UNLINK_PARENT_BUG and
        activate for the replacement function.
        * doc/posix-functions/unlink.texi: Mention the MacOS X 10.5 bug.

--- doc/posix-functions/unlink.texi.orig        Sat Mar 20 15:16:31 2010
+++ doc/posix-functions/unlink.texi     Sat Mar 20 11:24:55 2010
@@ -11,6 +11,9 @@
 @item
 Some systems mistakenly succeed on @code{unlink("link-to-file/")}:
 GNU/Hurd, FreeBSD 7.2, Solaris 9.
address@hidden
+On MacOS X 10.5.6, in a writable HFS mount, @code{unlink("..")} succeeds
+without doing anything.
 @end itemize
 
 Portability problems not fixed by Gnulib:
--- lib/unlink.c.orig   Sat Mar 20 15:16:31 2010
+++ lib/unlink.c        Sat Mar 20 11:53:49 2010
@@ -80,6 +80,16 @@
         }
     }
   if (!result)
-    result = unlink (name);
+    {
+#if UNLINK_PARENT_BUG
+      if (len >= 2 && name[len - 1] == '.' && name[len - 2] == '.'
+          && (len == 2 || ISSLASH (name[len - 3])))
+        {
+          errno = EISDIR; /* could also use EPERM */
+          return -1;
+        }
+#endif
+      result = unlink (name);
+    }
   return result;
 }
--- m4/unlink.m4.orig   Sat Mar 20 15:16:31 2010
+++ m4/unlink.m4        Sat Mar 20 15:16:14 2010
@@ -1,4 +1,4 @@
-# unlink.m4 serial 3
+# unlink.m4 serial 4
 dnl Copyright (C) 2009, 2010 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,9 +8,10 @@
 [
   AC_REQUIRE([gl_AC_DOS])
   AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
+  AC_REQUIRE([AC_CANONICAL_HOST])
   dnl Detect Solaris 9 and FreeBSD 7.2 bug.
   AC_CACHE_CHECK([whether unlink honors trailing slashes],
-    [gl_cv_func_unlink_works],
+    [gl_cv_func_unlink_honors_slashes],
     [touch conftest.file
      # Assume that if we have lstat, we can also check symlinks.
      if test $ac_cv_func_lstat = yes; then
@@ -25,10 +26,70 @@
       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"])
+      [gl_cv_func_unlink_honors_slashes=yes],
+      [gl_cv_func_unlink_honors_slashes=no],
+      [gl_cv_func_unlink_honors_slashes="guessing no"])
      rm -f conftest.file conftest.lnk])
-  if test x"$gl_cv_func_unlink_works" != xyes; then
+  dnl Detect MacOS X 10.5.6 bug: On read-write HFS mounts, unlink("..") or
+  dnl unlink("../..") succeeds without doing anything.
+  AC_CACHE_CHECK([whether unlink of a parent directory fails is it should],
+    [gl_cv_func_unlink_parent_fails],
+    [case "$host_os" in
+       darwin*)
+         if {
+              # Use the mktemp program if available. If not available, hide 
the error
+              # message.
+              tmp=`(umask 077 && mktemp -d /tmp/gtXXXXXX) 2>/dev/null` &&
+              test -n "$tmp" && test -d "$tmp"
+            } ||
+            {
+              # Use a simple mkdir command. It is guaranteed to fail if the 
directory
+              # already exists.  $RANDOM is bash specific and expands to empty 
in shells
+              # other than bash, ksh and zsh.  Its use does not increase 
security;
+              # rather, it minimizes the probability of failure in a very 
cluttered /tmp
+              # directory.
+              tmp=/tmp/gt$$-$RANDOM
+              (umask 077 && mkdir "$tmp")
+            }; then
+           mkdir "$tmp/subdir"
+           export tmp
+           AC_RUN_IFELSE(
+             [AC_LANG_SOURCE([[
+                #include <stdlib.h>
+                #include <unistd.h>
+                int main ()
+                {
+                  if (chdir (getenv ("tmp")) != 0)
+                    return 1;
+                  return unlink ("..") == 0;
+                }
+              ]])],
+             [gl_cv_func_unlink_parent_fails=yes],
+             [gl_cv_func_unlink_parent_fails=no],
+             [gl_cv_func_unlink_parent_fails="guessing no"])
+           rm -rf "$tmp"
+           unset tmp
+         else
+           gl_cv_func_unlink_parent_fails="guessing no"
+         fi
+         ;;
+       *)
+         gl_cv_func_unlink_parent_fails="guessing yes"
+         ;;
+     esac
+    ])
+  case "$gl_cv_func_unlink_parent_fails" in
+    *no)
+      AC_DEFINE([UNLINK_PARENT_BUG], [1],
+        [Define to 1 if unlink() on a parent directory may succeed])
+      ;;
+  esac
+  if test "$gl_cv_func_unlink_honors_slashes" != yes \
+     || { case "$gl_cv_func_unlink_parent_fails" in
+            *yes) false;;
+            *no) true;;
+          esac
+        }; then
     REPLACE_UNLINK=1
     AC_LIBOBJ([unlink])
   fi