Re: [bug #17900] rm -f should ignore ENOTDIR

[Jim Meyering]

Here's the patch I've checked in:
[but there's one small remaining patch coming right up...]

2006-10-03  Jim Meyering  <address@hidden>

        With --force (-f), rm no longer fails for ENOTDIR.
        * src/remove.c (ignorable_missing): New function.
        Use it everywhere, rather than open-coding the test.
        Andreas Schwab reported the ENOTDIR problem.
        (ignorable_missing): Similarly, don't fail for ENAMETOOLONG.
        * NEWS: Mention the bug fix.
        * tests/rm/ignorable: New file.  Test for the ENOTDIR case.
        * tests/rm/ignore-name-too-long: New file. Test for ENAMETOOLONG.
        * tests/rm/Makefile.am (TESTS): Add the new file names.

Index: src/remove.c
===================================================================
RCS file: /fetish/cu/src/remove.c,v
retrieving revision 1.162
diff -u -r1.162 remove.c
--- src/remove.c        29 Sep 2006 15:58:05 -0000      1.162
+++ src/remove.c        3 Oct 2006 10:37:01 -0000
@@ -899,7 +899,7 @@
          return RM_OK;                                                 \
        }                                                               \
                                                                        \
-      if (errno == ENOENT && (X)->ignore_missing_files)                        
\
+      if (ignorable_missing (X, errno))                                        
\
        return RM_OK;                                                   \
     }                                                                  \
   while (0)
@@ -915,7 +915,7 @@
          return RM_OK;                                 \
        }                                               \
                                                        \
-      if (errno == ENOENT && (X)->ignore_missing_files)        \
+      if (ignorable_missing (X, errno))                        \
        return RM_OK;                                   \
                                                        \
       if (errno == ENOTEMPTY || errno == EEXIST)       \
@@ -923,6 +923,32 @@
     }                                                  \
   while (0)

+/* When a function like unlink, rmdir, or fstatat fails with an errno
+   value of ERRNUM, return true if the specified file system object
+   is guaranteed not to exist;  otherwise, return false.  */
+static inline bool
+nonexistent_file_errno (int errnum)
+{
+  /* Do not include ELOOP here, since the specified file may indeed
+     exist, but be (in)accessible only via too long a symlink chain.  */
+  switch (errnum)
+    {
+    case ENAMETOOLONG:
+    case ENOENT:
+    case ENOTDIR:
+      return true;
+    default:
+      return false;
+    }
+}
+
+/* Encapsulate the test for whether the errno value, ERRNUM, is ignorable.  */
+static inline bool
+ignorable_missing (struct rm_options const *x, int errnum)
+{
+  return x->ignore_missing_files && nonexistent_file_errno (errnum);
+}
+
 /* Remove the file or directory specified by FILENAME.
    Return RM_OK if it is removed, and RM_ERROR or RM_USER_DECLINED if not.
    But if FILENAME specifies a non-empty directory, return RM_NONEMPTY_DIR. */
@@ -1014,7 +1040,7 @@
            {
              if (fstatat (fd_cwd, filename, st, AT_SYMLINK_NOFOLLOW))
                {
-                 if (errno == ENOENT && x->ignore_missing_files)
+                 if (ignorable_missing (x, errno))
                    return RM_OK;

                  error (0, errno, _("cannot remove %s"),
@@ -1195,7 +1221,7 @@

                /* CAUTION: this test and diagnostic are identical to
                   those following the other use of fd_to_subdirp.  */
-               if (errno == ENOENT && x->ignore_missing_files)
+               if (ignorable_missing (x, errno))
                  {
                    /* With -f, don't report "file not found".  */
                  }
@@ -1281,7 +1307,7 @@
     {
       /* CAUTION: this test and diagnostic are identical to
         those following the other use of fd_to_subdirp.  */
-      if (errno == ENOENT && x->ignore_missing_files)
+      if (ignorable_missing (x, errno))
        {
          /* With -f, don't report "file not found".  */
        }
@@ -1426,7 +1452,7 @@
     {
       if (cache_fstatat (AT_FDCWD, filename, &st, AT_SYMLINK_NOFOLLOW) != 0)
        {
-         if (errno == ENOENT && x->ignore_missing_files)
+         if (ignorable_missing (x, errno))
            return RM_OK;
          error (0, errno, _("cannot remove %s"), quote (filename));
          return RM_ERROR;
Index: NEWS
===================================================================
RCS file: /fetish/cu/NEWS,v
retrieving revision 1.436
diff -u -r1.436 NEWS
--- NEWS        2 Oct 2006 11:47:48 -0000       1.436
+++ NEWS        3 Oct 2006 10:36:19 -0000
@@ -2,6 +2,12 @@

 * Major changes in release 6.4-cvs (2006-??-??) [?????]

+** Bug fixes
+
+  With --force (-f), rm no longer fails for ENOTDIR or ENAMETOOLONG.
+  For example, "rm -f existing-non-directory/anything" now exits
+  successfully, ignoring the error about a nonexistent file.
+

 * Major changes in release 6.3 (2006-09-30) [stable]

Index: tests/rm/ignorable
===================================================================
RCS file: tests/rm/ignorable
diff -N tests/rm/ignorable
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/rm/ignorable  3 Oct 2006 12:29:29 -0000
@@ -0,0 +1,49 @@
+#!/bin/sh
+# Ensure that rm -f existing-non-dir/anything exits successfully
+
+# Copyright (C) 2006 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 2 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, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  rm --version
+fi
+
+PRIV_CHECK_ARG=require-non-root . $srcdir/../priv-check
+
+pwd=`pwd`
+t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp=$t0/$$
+trap 'status=$?; cd $pwd; chmod -R u+rwx $t0; rm -rf $t0 && exit $status' 0
+trap '(exit $?); exit $?' 1 2 13 15
+
+framework_failure=0
+mkdir -p $tmp || framework_failure=1
+cd $tmp || framework_failure=1
+touch existing-non-dir || framework_failure=1
+
+if test $framework_failure = 1; then
+  echo "$0: failure in testing framework" 1>&2
+  (exit 1); exit 1
+fi
+
+fail=0
+
+# With coreutils-6.3, this would exit nonzero.  It should not.
+# Example from Andreas Schwab.
+rm -f existing-non-dir/f > out 2>&1 || fail=1
+
+(exit $fail); exit $fail
Index: tests/rm/ignore-name-too-long
===================================================================
RCS file: tests/rm/ignore-name-too-long
diff -N tests/rm/ignore-name-too-long
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/rm/ignore-name-too-long       3 Oct 2006 12:37:51 -0000
@@ -0,0 +1,48 @@
+#!/bin/sh
+# Ensure that rm -f name_longer_than_FILENAME_MAX exits successfully
+
+# Copyright (C) 2006 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 2 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, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  rm --version
+fi
+
+PRIV_CHECK_ARG=require-non-root . $srcdir/../priv-check
+
+pwd=`pwd`
+t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp=$t0/$$
+trap 'status=$?; cd $pwd; chmod -R u+rwx $t0; rm -rf $t0 && exit $status' 0
+trap '(exit $?); exit $?' 1 2 13 15
+
+framework_failure=0
+mkdir -p $tmp || framework_failure=1
+cd $tmp || framework_failure=1
+
+if test $framework_failure = 1; then
+  echo "$0: failure in testing framework" 1>&2
+  (exit 1); exit 1
+fi
+
+fail=0
+
+# With coreutils-6.3, this would exit nonzero.  It should not.
+long_name=$(printf %0513d 0)
+rm -f $long_name > out 2>&1 || fail=1
+
+(exit $fail); exit $fail
Index: tests/rm/Makefile.am
===================================================================
RCS file: /fetish/cu/tests/rm/Makefile.am,v
retrieving revision 1.38
diff -u -r1.38 Makefile.am
--- tests/rm/Makefile.am        29 Sep 2006 09:54:25 -0000      1.38
+++ tests/rm/Makefile.am        3 Oct 2006 12:38:10 -0000
@@ -21,6 +21,8 @@
 AUTOMAKE_OPTIONS = 1.1 gnits

 TESTS = \
+  ignore-name-too-long \
+  ignorable \
   readdir-bug \
   empty-inacc \
   dir-nonrecur \