mingw remove bug

[Eric Blake]

mingw remove() complies with C89, but not with POSIX.  I'll be committing this 
shortly (actually, first I probably need to commit a stat module, to make up 
for the fact that both mingw rmdir("dir") and rmdir("dir/") can succeed, even 
when one of stat("dir") and stat("dir/") fails, depending on which directory 
was passed in).


From: Eric Blake <address@hidden>
Date: Mon, 14 Sep 2009 16:57:55 -0600
Subject: [PATCH] remove: new module

* modules/remove: New file.
* lib/remove.c: Likewise.
* m4/remove.m4 (gl_FUNC_REMOVE): Likewise.
* m4/stdio_h.m4 (gl_STDIO_H_DEFAULTS): Add witnesses.
* modules/stdio (Makefile.am): Use them.
* lib/stdio.in.h (remove): Declare replacement.
* MODULES.html.sh (systems lacking POSIX:2008): Mention module.
* doc/posix-functions/remove.texi (remove): Likewise.
* modules/remove-tests: New test.
* tests/test-remove.c: Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                       |   14 +++++
 MODULES.html.sh                 |    1 +
 doc/posix-functions/remove.texi |    5 ++-
 lib/remove.c                    |   63 ++++++++++++++++++++++++
 lib/stdio.in.h                  |   14 +++++
 m4/remove.m4                    |   32 ++++++++++++
 m4/stdio_h.m4                   |    4 +-
 modules/remove                  |   26 ++++++++++
 modules/remove-tests            |   12 +++++
 modules/stdio                   |    2 +
 tests/test-remove.c             |  102 +++++++++++++++++++++++++++++++++++++++
 11 files changed, 273 insertions(+), 2 deletions(-)
 create mode 100644 lib/remove.c
 create mode 100644 m4/remove.m4
 create mode 100644 modules/remove
 create mode 100644 modules/remove-tests
 create mode 100644 tests/test-remove.c

diff --git a/ChangeLog b/ChangeLog
index dc32b4d..1c5b190 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2009-09-14  Eric Blake  <address@hidden>
+
+       remove: new module
+       * modules/remove: New file.
+       * lib/remove.c: Likewise.
+       * m4/remove.m4 (gl_FUNC_REMOVE): Likewise.
+       * m4/stdio_h.m4 (gl_STDIO_H_DEFAULTS): Add witnesses.
+       * modules/stdio (Makefile.am): Use them.
+       * lib/stdio.in.h (remove): Declare replacement.
+       * MODULES.html.sh (systems lacking POSIX:2008): Mention module.
+       * doc/posix-functions/remove.texi (remove): Likewise.
+       * modules/remove-tests: New test.
+       * tests/test-remove.c: Likewise.
+
 2009-09-13  Jim Meyering  <address@hidden>

        posixtm: don't reject a time that specify "60" as the number of seconds
diff --git a/MODULES.html.sh b/MODULES.html.sh
index 76741b3..6cfd968 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -2319,6 +2319,7 @@ func_all_modules ()
   func_module realloc-posix
   func_module recv
   func_module recvfrom
+  func_module remove
   func_module sched
   func_module select
   func_module send
diff --git a/doc/posix-functions/remove.texi b/doc/posix-functions/remove.texi
index 3995006..6d687d8 100644
--- a/doc/posix-functions/remove.texi
+++ b/doc/posix-functions/remove.texi
@@ -4,10 +4,13 @@ remove

 POSIX specification: @url
{http://www.opengroup.org/onlinepubs/9699919799/functions/remove.html}

-Gnulib module: ---
+Gnulib module: remove

 Portability problems fixed by Gnulib:
 @itemize
address@hidden
+This function does not remove empty directories on some platforms:
+mingw.
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/lib/remove.c b/lib/remove.c
new file mode 100644
index 0000000..90d5b2f
--- /dev/null
+++ b/lib/remove.c
@@ -0,0 +1,63 @@
+/* Remove a file or directory.
+   Copyright (C) 2009 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* written by Eric Blake */
+
+#include <config.h>
+
+#include <stdio.h>
+
+#include <errno.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#undef remove
+
+/* Remove NAME from the file system.  */
+int
+rpl_remove (char const *name)
+{
+  /* Mingw remove() fails with EPERM on empty directories.  */
+  struct stat st;
+  size_t len = strlen (name);
+  if (!len)
+    {
+      errno = ENOENT;
+      return -1;
+    }
+
+  if (lstat (name, &st) == 0 && S_ISDIR (st.st_mode))
+    {
+      /* Mingw rmdir("empty/.") mistakenly succeeds.  */
+      while (ISSLASH (name[len - 1]))
+        len--;
+      if (name[len - 1] == '.' && (1 == len || ISSLASH (name[len - 2])))
+        {
+          errno = EINVAL;
+          return -1;
+        }
+      return rmdir (name);
+    }
+  /* Mingw remove("file/") fails with EINVAL, instead of the required
+     ENOTDIR.  */
+  if (ISSLASH (name[len - 1]))
+    {
+      errno = ENOTDIR;
+      return -1;
+    }
+  return remove (name);
+}
diff --git a/lib/stdio.in.h b/lib/stdio.in.h
index 27cd305..35109c3 100644
--- a/lib/stdio.in.h
+++ b/lib/stdio.in.h
@@ -416,6 +416,20 @@ extern int putchar (int c);
 extern int puts (const char *string);
 #endif

+#if @GNULIB_REMOVE@
+# if @REPLACE_REMOVE@
+#  undef remove
+#  define remove rpl_remove
+extern int remove (const char *name);
+# endif
+#elif defined GNULIB_POSIXCHECK
+# undef remove
+# define remove(n)                                        \
+   (GL_LINK_WARNING ("remove cannot handle directories on some platforms - " \
+                     "use gnulib module remove for more portability"), \
+    remove (n))
+#endif
+
 #if @GNULIB_RENAME@
 # if @REPLACE_RENAME@
 #  undef rename
diff --git a/m4/remove.m4 b/m4/remove.m4
new file mode 100644
index 0000000..b377c59
--- /dev/null
+++ b/m4/remove.m4
@@ -0,0 +1,32 @@
+# remove.m4 serial 1
+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,
+dnl with or without modifications, as long as this notice is preserved.
+
+AC_DEFUN([gl_FUNC_REMOVE],
+[
+  AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
+  AC_REQUIRE([gl_AC_DOS])
+  AC_REQUIRE([gl_STDIO_H_DEFAULTS])
+  dnl C89 requires remove(), but only POSIX requires it to handle
+  dnl directories.  On mingw, it fails with EPERM.
+  AC_CACHE_CHECK([whether remove handles directories],
+      [gl_cv_func_remove_works],
+      [mkdir conftest.dir
+       AC_RUN_IFELSE(
+         [AC_LANG_PROGRAM(
+           [[#include <stdio.h>
+]], [[return remove ("conftest.dir");]])],
+         [gl_cv_func_remove_works=yes], [gl_cv_func_remove_works=no],
+         [case $host_os in
+            mingw*) gl_cv_func_remove_works="guessing no";;
+            *) gl_cv_func_remove_works="guessing yes";;
+          esac])
+       rm -rf conftest.dir])
+  case $gl_cv_func_remove_works in
+    *yes) ;;
+    *) REPLACE_REMOVE=1
+       AC_LIBOBJ([remove]);;
+  esac
+])
diff --git a/m4/stdio_h.m4 b/m4/stdio_h.m4
index ac5e20a..01af04d 100644
--- a/m4/stdio_h.m4
+++ b/m4/stdio_h.m4
@@ -1,4 +1,4 @@
-# stdio_h.m4 serial 18
+# stdio_h.m4 serial 19
 dnl Copyright (C) 2007-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,
@@ -67,6 +67,7 @@ AC_DEFUN([gl_STDIO_H_DEFAULTS],
   GNULIB_PUTC=0;                 AC_SUBST([GNULIB_PUTC])
   GNULIB_PUTCHAR=0;              AC_SUBST([GNULIB_PUTCHAR])
   GNULIB_PUTS=0;                 AC_SUBST([GNULIB_PUTS])
+  GNULIB_REMOVE=0;               AC_SUBST([GNULIB_REMOVE])
   GNULIB_RENAME=0;               AC_SUBST([GNULIB_RENAME])
   GNULIB_SNPRINTF=0;             AC_SUBST([GNULIB_SNPRINTF])
   GNULIB_SPRINTF_POSIX=0;        AC_SUBST([GNULIB_SPRINTF_POSIX])
@@ -107,6 +108,7 @@ AC_DEFUN([gl_STDIO_H_DEFAULTS],
   REPLACE_PERROR=0;              AC_SUBST([REPLACE_PERROR])
   REPLACE_POPEN=0;               AC_SUBST([REPLACE_POPEN])
   REPLACE_PRINTF=0;              AC_SUBST([REPLACE_PRINTF])
+  REPLACE_REMOVE=0;              AC_SUBST([REPLACE_REMOVE])
   REPLACE_RENAME=0;              AC_SUBST([REPLACE_RENAME])
   REPLACE_SNPRINTF=0;            AC_SUBST([REPLACE_SNPRINTF])
   REPLACE_SPRINTF=0;             AC_SUBST([REPLACE_SPRINTF])
diff --git a/modules/remove b/modules/remove
new file mode 100644
index 0000000..fa9ba18
--- /dev/null
+++ b/modules/remove
@@ -0,0 +1,26 @@
+Description:
+remove(): remove a file or directory
+
+Files:
+lib/remove.c
+m4/dos.m4
+m4/remove.m4
+
+Depends-on:
+lstat
+stdio
+
+configure.ac:
+gl_FUNC_REMOVE
+gl_STDIO_MODULE_INDICATOR([remove])
+
+Makefile.am:
+
+Include:
+<stdio.h>
+
+License:
+LGPL
+
+Maintainer:
+Eric Blake
diff --git a/modules/remove-tests b/modules/remove-tests
new file mode 100644
index 0000000..fdc26c2
--- /dev/null
+++ b/modules/remove-tests
@@ -0,0 +1,12 @@
+Files:
+tests/test-remove.c
+
+Depends-on:
+sys_stat
+
+configure.ac:
+AC_CHECK_FUNCS_ONCE([symlink])
+
+Makefile.am:
+TESTS += test-remove
+check_PROGRAMS += test-remove
diff --git a/modules/stdio b/modules/stdio
index eb58269..1cd3212 100644
--- a/modules/stdio
+++ b/modules/stdio
@@ -52,6 +52,7 @@ stdio.h: stdio.in.h
              -e 's|@''GNULIB_PUTC''@|$(GNULIB_PUTC)|g' \
              -e 's|@''GNULIB_PUTCHAR''@|$(GNULIB_PUTCHAR)|g' \
              -e 's|@''GNULIB_PUTS''@|$(GNULIB_PUTS)|g' \
+             -e 's|@''GNULIB_REMOVE''@|$(GNULIB_REMOVE)|g' \
              -e 's|@''GNULIB_RENAME''@|$(GNULIB_RENAME)|g' \
              -e 's|@''GNULIB_SNPRINTF''@|$(GNULIB_SNPRINTF)|g' \
              -e 's|@''GNULIB_SPRINTF_POSIX''@|$(GNULIB_SPRINTF_POSIX)|g' \
@@ -89,6 +90,7 @@ stdio.h: stdio.in.h
              -e 's|@''REPLACE_PERROR''@|$(REPLACE_PERROR)|g' \
              -e 's|@''REPLACE_POPEN''@|$(REPLACE_POPEN)|g' \
              -e 's|@''REPLACE_PRINTF''@|$(REPLACE_PRINTF)|g' \
+             -e 's|@''REPLACE_REMOVE''@|$(REPLACE_REMOVE)|g' \
              -e 's|@''REPLACE_RENAME''@|$(REPLACE_RENAME)|g' \
              -e 's|@''REPLACE_SNPRINTF''@|$(REPLACE_SNPRINTF)|g' \
              -e 's|@''REPLACE_SPRINTF''@|$(REPLACE_SPRINTF)|g' \
diff --git a/tests/test-remove.c b/tests/test-remove.c
new file mode 100644
index 0000000..2571d34
--- /dev/null
+++ b/tests/test-remove.c
@@ -0,0 +1,102 @@
+/* Tests of remove.
+   Copyright (C) 2009 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Eric Blake <address@hidden>, 2009.  */
+
+#include <config.h>
+
+#include <stdio.h>
+
+#include <fcntl.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#if !HAVE_SYMLINK
+# define symlink(a,b) (-1)
+#endif
+
+#define ASSERT(expr) \
+  do                                                                         \
+    {                                                                        \
+      if (!(expr))                                                           \
+       {                                                                    \
+         fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__);  \
+         fflush (stderr);                                                   \
+         abort ();                                                          \
+       }                                                                    \
+    }                                                                        \
+  while (0)
+
+int
+main ()
+{
+  /* Setup.  */
+  ASSERT (mkdir ("test-remove.dir", 0700) == 0);
+  ASSERT (close (creat ("test-remove.dir/file", 0600)) == 0);
+
+  /* Basic error conditions.  */
+  errno = 0;
+  ASSERT (remove ("") == -1);
+  ASSERT (errno == ENOENT);
+  errno = 0;
+  ASSERT (remove ("test-remove.nosuch") == -1);
+  ASSERT (errno == ENOENT);
+  errno = 0;
+  ASSERT (remove (".") == -1);
+  ASSERT (errno == EINVAL);
+  errno = 0;
+  ASSERT (remove ("..") == -1);
+  ASSERT (errno == EINVAL || errno == EEXIST || errno == ENOTEMPTY);
+  errno = 0;
+  ASSERT (remove ("test-remove.dir/file/") == -1);
+  ASSERT (errno == ENOTDIR);
+
+  /* Non-empty directory.  */
+  errno = 0;
+  ASSERT (remove ("test-remove.dir") == -1);
+  ASSERT (errno == EEXIST || errno == ENOTEMPTY);
+
+  /* Non-directory.  */
+  ASSERT (remove ("test-remove.dir/file") == 0);
+
+  /* Empty directory.  */
+  errno = 0;
+  ASSERT (remove ("test-remove.dir/./") == -1);
+  ASSERT (errno == EINVAL);
+  ASSERT (remove ("test-remove.dir") == 0);
+
+  /* Test symlink behavior.  Specifying trailing slash should remove
+     referent directory, not symlink.  */
+  if (symlink ("test-remove.dir", "test-remove.link") != 0)
+    {
+      fputs ("skipping test: symlinks not supported on this filesystem\n",
+             stderr);
+      return 77;
+    }
+  ASSERT (mkdir ("test-remove.dir", 0700) == 0);
+  ASSERT (remove ("test-remove.link/") == 0);
+  {
+    struct stat st;
+    ASSERT (lstat ("test-remove.link", &st) == 0);
+    ASSERT (S_ISLNK (st.st_mode));
+  }
+  ASSERT (remove ("test-remove.link") == 0);
+
+  return 0;
+}
-- 
1.6.4.2