FYI: "rm --interactive=never F" no longer prompts for an unwritable F

[Jim Meyering]

I noticed that rm's new --interactive=[WHEN] option didn't work properly
in some cases.  For example, this use of rm would prompt, contradicting
the semantics of --interactive=never:

  $ touch unwritable
  $ chmod -w unwritable
  $ rm --interactive=never unwritable
  rm: remove write-protected regular empty file `unwritable'?

With the fix below, it is no longer interactive.

        Make "rm --interactive=never ..." never prompt.
        * NEWS: Mention this.
        * src/remove.h (enum rm_interactive): New ternary type.
        (struct rm_options) [interactive]: Use it, here -- rather than bool.
        * src/remove.c (prompt): Reflect type change.
        * src/mv.c (rm_option_init): Initialize to RMI_NEVER now.
        * src/rm.c (main): Add a FIXME comment for '-d' option.
        Adapt to type change of rm_options.interactive.

        * tests/rm/i-never: New file.  Test for the above fix.
        * tests/rm/Makefile.am (TESTS): Add i-never.

Here's a link to the same change-set:
  http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commit;h=0928c2414a

Signed-off-by: Jim Meyering <address@hidden>
---
 NEWS                 |    2 +
 ChangeLog            |   14 +++++++++++++
 src/mv.c             |    4 +-
 src/remove.c         |   10 ++++++--
 src/remove.h         |   13 ++++++++++-
 src/rm.c             |   18 ++++++++--------
 tests/rm/Makefile.am |    3 +-
 tests/rm/i-never     |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 100 insertions(+), 17 deletions(-)

diff --git a/NEWS b/NEWS
index 76f90d3..a30b17e 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,8 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   "rm -rf /etc/passwd" (run by non-root) now prints a diagnostic.
   Before it would print nothing.

+  "rm --interactive=never F" no longer prompts for an unwritable F
+

 * Noteworthy changes in release 6.7 (2006-12-08) [stable]

diff --git a/ChangeLog b/ChangeLog
index b82902a..7a80dbf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2007-01-17  Jim Meyering  <address@hidden>
+
+       Make "rm --interactive=never ..." never prompt.
+       * NEWS: Mention this.
+       * src/remove.h (enum rm_interactive): New ternary type.
+       (struct rm_options) [interactive]: Use it, here -- rather than bool.
+       * src/remove.c (prompt): Reflect type change.
+       * src/mv.c (rm_option_init): Initialize to RMI_NEVER now.
+       * src/rm.c (main): Add a FIXME comment for '-d' option.
+       Adapt to type change of rm_options.interactive.
+
+       * tests/rm/i-never: New file.  Test for the above fix.
+       * tests/rm/Makefile.am (TESTS): Add i-never.
+
 2007-01-15  Jim Meyering  <address@hidden>

        * bootstrap (gnulib_tool): When adding to .cvsignore and .gitignore,
diff --git a/src/mv.c b/src/mv.c
index 03e96e5..1d1ddda 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -1,5 +1,5 @@
 /* mv -- move or rename files
-   Copyright (C) 86, 89, 90, 91, 1995-2006 Free Software Foundation, Inc.
+   Copyright (C) 86, 89, 90, 91, 1995-2007 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
@@ -98,7 +98,7 @@ rm_option_init (struct rm_options *x)

   /* Should we prompt for removal, too?  No.  Prompting for the `move'
      part is enough.  It implies removal.  */
-  x->interactive = 0;
+  x->interactive = RMI_NEVER;
   x->stdin_tty = false;

   x->verbose = false;
diff --git a/src/remove.c b/src/remove.c
index fbe7204..97184eb 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -798,10 +798,14 @@ prompt (int fd_cwd, Dirstack_state const *ds, char const 
*filename,

   *is_empty = T_UNKNOWN;

-  if (((!x->ignore_missing_files & (x->interactive | x->stdin_tty))
+  if (x->interactive == RMI_NEVER)
+    return RM_OK;
+
+  if (((!x->ignore_missing_files & ((x->interactive == RMI_ALWAYS)
+                                   | x->stdin_tty))
        && (write_protected = write_protected_non_symlink (fd_cwd, filename,
                                                          ds, sbuf)))
-      || x->interactive)
+      || x->interactive == RMI_ALWAYS)
     {
       if (cache_fstatat (fd_cwd, filename, sbuf, AT_SYMLINK_NOFOLLOW) != 0)
        {
@@ -821,7 +825,7 @@ prompt (int fd_cwd, Dirstack_state const *ds, char const 
*filename,
       /* Using permissions doesn't make sense for symlinks.  */
       if (S_ISLNK (sbuf->st_mode))
        {
-         if ( ! x->interactive)
+         if (x->interactive != RMI_ALWAYS)
            return RM_OK;
          write_protected = false;
        }
diff --git a/src/remove.h b/src/remove.h
index 2dc6176..ae01e3c 100644
--- a/src/remove.h
+++ b/src/remove.h
@@ -1,6 +1,6 @@
 /* Remove directory entries.

-   Copyright (C) 1998, 2000, 2002, 2003, 2004, 2005, 2006 Free
+   Copyright (C) 1998, 2000, 2002, 2003, 2004, 2005, 2006, 2007 Free
    Software Foundation, Inc.

    This program is free software; you can redistribute it and/or modify
@@ -22,13 +22,22 @@

 # include "dev-ino.h"

+enum rm_interactive
+{
+  /* Start with any number larger than 1, so that any legacy tests
+     against values of 0 or 1 will fail.  */
+  RMI_ALWAYS = 3,
+  RMI_SOMETIMES,
+  RMI_NEVER
+};
+
 struct rm_options
 {
   /* If true, ignore nonexistent files.  */
   bool ignore_missing_files;

   /* If true, query the user about whether to remove each file.  */
-  bool interactive;
+  enum rm_interactive interactive;

   /* If true, do not traverse into (or remove) any directory that is
      on a file system (i.e., that has a different device number) other
diff --git a/src/rm.c b/src/rm.c
index 364a21c..81f81ec 100644
--- a/src/rm.c
+++ b/src/rm.c
@@ -1,5 +1,5 @@
 /* `rm' file deletion utility for GNU.
-   Copyright (C) 88, 90, 91, 1994-2006 Free Software Foundation, Inc.
+   Copyright (C) 88, 90, 91, 1994-2007 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
@@ -213,7 +213,7 @@ static void
 rm_option_init (struct rm_options *x)
 {
   x->ignore_missing_files = false;
-  x->interactive = false;
+  x->interactive = RMI_SOMETIMES;
   x->one_file_system = false;
   x->recursive = false;
   x->root_dev_ino = NULL;
@@ -249,25 +249,25 @@ main (int argc, char **argv)
        {
        case 'd':
          /* Ignore this option, for backward compatibility with
-            coreutils 5.92.  Some time after 2005, we'll change this
+            coreutils 5.92.  FIXME: Some time after 2005, change this
             to report an error (or perhaps behave like FreeBSD does)
             instead of ignoring the option.  */
          break;

        case 'f':
-         x.interactive = false;
+         x.interactive = RMI_NEVER;
          x.ignore_missing_files = true;
          prompt_once = false;
          break;

        case 'i':
-         x.interactive = true;
+         x.interactive = RMI_ALWAYS;
          x.ignore_missing_files = false;
          prompt_once = false;
          break;

        case 'I':
-         x.interactive = false;
+         x.interactive = RMI_NEVER;
          x.ignore_missing_files = false;
          prompt_once = true;
          break;
@@ -288,18 +288,18 @@ main (int argc, char **argv)
            switch (i)
              {
              case interactive_never:
-               x.interactive = false;
+               x.interactive = RMI_NEVER;
                prompt_once = false;
                break;

              case interactive_once:
-               x.interactive = false;
+               x.interactive = RMI_SOMETIMES;
                x.ignore_missing_files = false;
                prompt_once = true;
                break;

              case interactive_always:
-               x.interactive = true;
+               x.interactive = RMI_ALWAYS;
                x.ignore_missing_files = false;
                prompt_once = false;
                break;
diff --git a/tests/rm/Makefile.am b/tests/rm/Makefile.am
index ff31f75..4c212b9 100644
--- a/tests/rm/Makefile.am
+++ b/tests/rm/Makefile.am
@@ -1,6 +1,6 @@
 # Make coreutils tests for "rm".                       -*-Makefile-*-

-# Copyright (C) 1997, 1998, 2000, 2001, 2002, 2003, 2004, 2005, 2006
+# Copyright (C) 1997, 1998, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007
 # Free Software Foundation, Inc.

 # This program is free software; you can redistribute it and/or modify
@@ -21,6 +21,7 @@
 AUTOMAKE_OPTIONS = 1.1 gnits

 TESTS = \
+  i-never \
   fail-eacces \
   one-file-system \
   ignorable \
diff --git a/tests/rm/i-never b/tests/rm/i-never
new file mode 100755
index 0000000..f20e90e
--- /dev/null
+++ b/tests/rm/i-never
@@ -0,0 +1,53 @@
+#!/bin/sh
+# Ensure that rm --interactive=never works does not prompt, even for
+# an unwritable file.
+
+# Copyright (C) 2007 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 f || framework_failure=1
+chmod 0 f || framework_failure=1
+touch exp || framework_failure=1
+
+if test $framework_failure = 1; then
+  echo "$0: failure in testing framework" 1>&2
+  (exit 1); exit 1
+fi
+
+fail=0
+
+rm --interactive=never f > out || fail=1
+
+cmp out exp || fail=1
+test $fail = 1 && diff out exp 2> /dev/null
+
+(exit $fail); exit $fail
--
1.5.0.rc1.g696b