bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: coreutils 'rm /' incompatibility with 2006-06-30 draft POSIX


From: Paul Eggert
Subject: Re: coreutils 'rm /' incompatibility with 2006-06-30 draft POSIX
Date: Sat, 02 Sep 2006 20:49:22 -0700
User-agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux)

Jim Meyering <address@hidden> writes:

> You're welcome to make that change.

OK, I installed the following.  This turned into a bigger change than
I thought it would.  I discovered another problem, which is that the
POSIX spec says "rm -r ./" should fail (and likewise for ../) but GNU
rm tries to remove it.  Also, the simplest way to implement the change
involved an extra 'stat' at the start, which I wanted to avoid, so I
cached the stat result, and this propagated throughout much of
remove.c.

I hope the resulting code is clearer and/or more efficient (if not
please let me know).

Here's what I installed:

2006-09-02  Paul Eggert  <address@hidden>

        * NEWS: rm now rejects attempts to remove /, ./, and ../.
        * doc/coreutils.texi (Treating / specially): --preserve-root is
        now the default for rm.
        (rm invocation): Likewise.  Also, document that you can't
        remove `.' or `..'.  Use the POSIX term "root directory"
        rather than the more-ambiguous "file system root".
        * src/basename.c: Don't include dirname.h, since system.h does it now.
        * src/chmod.c: Likewise.
        * src/copy.c: Likewise.
        * src/cp.c: Likewise.
        * src/df.c: Likewise.
        * src/dircolors.c: Likewise.
        * src/dirname.c: Likewise.
        * src/du.c: Likewise.
        * src/install.c: Likewise.
        * src/ln.c: Likewise.
        * src/ls.c: Likewise.
        * src/mkdir.c: Likewise.
        * src/mv.c: Likewise.
        * src/remove.c: Likewise.
        * src/rm.c: Likewise.
        * src/rmdir.c: Likewise.
        * src/shred.c: Likewise.
        * src/split.c: Likewise.
        * src/su.c: Likewise.
        * src/system.h: Include "dirname.h", since dot_or_dotdot needs it
        now.
        (dot_or_dotdot): Succeed even if "." or ".." is followed by a
        slash.
        * src/rm.c (usage, main): --preserve-root is now the default.
        * src/remove.h: Fix comment.
        * src/remove.c (cache_fstatat, cache_stat_init): New functions.
        (cache_statted, cache_stat_ok): New functions.
        (write_protected_non_symlink): Remove struct stat ** buf_p arg,
        which is no longer needed with the new functions.  All callers
        changed.
        (prompt, is_dir_lstat, remove_entry, remove_dir):
        New struct stat * arg.  All callers changed.
        (write_protected_non_symlink, prompt, is_dir_lstat, remove_entry):
        (remove_cwd_entries, remove_dir, rm_1):
        Use and maintain the file status cache.
        (prompt, remove_entry): Omit the first "directory" in the diagnostic
        "Cannot remove directory `foo': is a directory".  This causes "rm"
        to pass a test case that it would otherwise fail now that it
        "knows" more about its argument.  I think the diagnostic is better
        without the first "directory" anyway.
        (prompt): Remove the no-longer-needed IS_DIR arg; all callers changed.
        (rm_1): Reject attempts to remove /, ./, or ../.
        * tests/rm/Makefile.am (TESTS): Add r-4.
        * tests/rm/r-4: New file.

Index: NEWS
===================================================================
RCS file: /fetish/cu/NEWS,v
retrieving revision 1.413
diff -p -u -r1.413 NEWS
--- NEWS        27 Aug 2006 19:46:26 -0000      1.413
+++ NEWS        3 Sep 2006 02:49:27 -0000
@@ -2,6 +2,12 @@ GNU coreutils NEWS                      
 
 * Major changes in release 6.2-cvs (2006-??-??) [unstable]
 
+** Changes in behavior
+
+  rm now rejects attempts to remove the root directory, e.g., `rm -fr /'
+  now fails without removing anything.  Likewise for file names ending
+  in `./' and `../'.
+
 ** Infrastructure changes
 
   Coreutils now uses gnulib via the gnulib-tool script.
Index: doc/coreutils.texi
===================================================================
RCS file: /fetish/cu/doc/coreutils.texi,v
retrieving revision 1.347
diff -p -u -r1.347 coreutils.texi
--- doc/coreutils.texi  17 Aug 2006 19:58:18 -0000      1.347
+++ doc/coreutils.texi  3 Sep 2006 02:49:29 -0000
@@ -1137,18 +1137,14 @@ or @option{-P} is specified.
 
 Certain commands can operate destructively on entire hierarchies.
 For example, if a user with appropriate privileges mistakenly runs
address@hidden -rf / tmp/junk} or @samp{cd /bin; rm -rf ../}, that may remove
address@hidden -rf / tmp/junk}, that may remove
 all files on the entire system.  Since there are so few
address@hidden you know of one, please write to @email{bug-coreutils@@gnu.org}.}
 legitimate uses for such a command,
address@hidden @command{rm} provides the @option{--preserve-root} option
-to make it so @command{rm} declines to operate on any directory
-that resolves to @file{/}.  The default is still to allow
address@hidden -rf /} to operate unimpeded.
-Another new option, @option{--no-preserve-root}, cancels the
-effect of any preceding @option{--preserve-root} option.
-Note that the @option{--preserve-root} behavior may become the default
-for @command{rm}.
address@hidden @command{rm} normally declines to operate on any directory
+that resolves to @file{/}.  If you really want to try to remove all
+the files on your system, you can use the @option{--no-preserve-root}
+option, but the default behavior, specified by the
address@hidden, is safer for most purposes.
 
 The commands @command{chgrp}, @command{chmod} and @command{chown}
 can also operate destructively on entire hierarchies, so they too
@@ -1156,7 +1152,11 @@ support these options.  Although, unlike
 actually unlink files, these commands are arguably more dangerous
 when operating recursively on @file{/}, since they often work much
 more quickly, and hence damage more files before an alert user can
-interrupt them.
+interrupt them.  Tradition and @acronym{POSIX} require these commands
+to operate recursively on @file{/}, so they default to
address@hidden, but using the @option{--preserve-root}
+option makes them safer for most purposes.  For convenience you can
+specify @option{--preserve-root} in an alias or in a shell function.
 
 @node Special built-in utilities
 @section Special built-in utilities
@@ -7717,6 +7717,9 @@ the @option{-f} or @option{--force} opti
 @command{rm} prompts the user for whether to remove the file.
 If the response is not affirmative, the file is skipped.
 
+Any attempt to remove a file whose last file name component is
address@hidden or @file{..} is rejected without any prompting.
+
 @emph{Warning}: If you use @command{rm} to remove a file, it is usually
 possible to recover the contents of that file.  If you want more assurance
 that the contents are truly unrecoverable, consider using @command{shred}.
@@ -7768,15 +7771,17 @@ Specifying @option{--interactive} and no
 @itemx --preserve-root
 @opindex --preserve-root
 @cindex root directory, disallow recursive destruction
-Fail upon any attempt to remove the file system root, @file{/},
+Fail upon any attempt to remove the root directory, @file{/},
 when used with the @option{--recursive} option.
-Without @option{--recursive}, this option has no effect.
+This is the default behavior.
 @xref{Treating / specially}.
 
 @itemx --no-preserve-root
 @opindex --no-preserve-root
 @cindex root directory, allow recursive destruction
-Cancel the effect of any preceding @option{--preserve-root} option.
+Do not treat @file{/} specially when removing recursively.
+This option is not recommended unless you really want to
+remove all the files on your computer.
 @xref{Treating / specially}.
 
 @item -r
@@ -8798,7 +8803,7 @@ during a recursive traversal, but see @o
 @itemx --preserve-root
 @opindex --preserve-root
 @cindex root directory, disallow recursive modification
-Fail upon any attempt to recursively change the file system root, @file{/}.
+Fail upon any attempt to recursively change the root directory, @file{/}.
 Without @option{--recursive}, this option has no effect.
 @xref{Treating / specially}.
 
@@ -8920,7 +8925,7 @@ during a recursive traversal, but see @o
 @itemx --preserve-root
 @opindex --preserve-root
 @cindex root directory, disallow recursive modification
-Fail upon any attempt to recursively change the file system root, @file{/}.
+Fail upon any attempt to recursively change the root directory, @file{/}.
 Without @option{--recursive}, this option has no effect.
 @xref{Treating / specially}.
 
@@ -9041,7 +9046,7 @@ changed.
 @itemx --preserve-root
 @opindex --preserve-root
 @cindex root directory, disallow recursive modification
-Fail upon any attempt to recursively change the file system root, @file{/}.
+Fail upon any attempt to recursively change the root directory, @file{/}.
 Without @option{--recursive}, this option has no effect.
 @xref{Treating / specially}.
 
Index: src/basename.c
===================================================================
RCS file: /fetish/cu/src/basename.c,v
retrieving revision 1.64
diff -p -u -r1.64 basename.c
--- src/basename.c      26 Mar 2006 11:59:58 -0000      1.64
+++ src/basename.c      3 Sep 2006 02:49:29 -0000
@@ -32,7 +32,6 @@
 
 #include "system.h"
 #include "long-options.h"
-#include "dirname.h"
 #include "error.h"
 #include "quote.h"
 
Index: src/chmod.c
===================================================================
RCS file: /fetish/cu/src/chmod.c,v
retrieving revision 1.117
diff -p -u -r1.117 chmod.c
--- src/chmod.c 17 Jul 2006 03:09:26 -0000      1.117
+++ src/chmod.c 3 Sep 2006 02:49:29 -0000
@@ -24,7 +24,6 @@
 
 #include "system.h"
 #include "dev-ino.h"
-#include "dirname.h"
 #include "error.h"
 #include "filemode.h"
 #include "modechange.h"
Index: src/copy.c
===================================================================
RCS file: /fetish/cu/src/copy.c,v
retrieving revision 1.210
diff -p -u -r1.210 copy.c
--- src/copy.c  28 Aug 2006 23:29:38 -0000      1.210
+++ src/copy.c  3 Sep 2006 02:49:29 -0000
@@ -35,7 +35,6 @@
 #include "buffer-lcm.h"
 #include "copy.h"
 #include "cp-hash.h"
-#include "dirname.h"
 #include "euidaccess.h"
 #include "error.h"
 #include "fcntl--.h"
Index: src/cp.c
===================================================================
RCS file: /fetish/cu/src/cp.c,v
retrieving revision 1.222
diff -p -u -r1.222 cp.c
--- src/cp.c    28 Aug 2006 23:29:38 -0000      1.222
+++ src/cp.c    3 Sep 2006 02:49:29 -0000
@@ -28,7 +28,6 @@
 #include "copy.h"
 #include "cp-hash.h"
 #include "error.h"
-#include "dirname.h"
 #include "filenamecat.h"
 #include "lchmod.h"
 #include "quote.h"
Index: src/df.c
===================================================================
RCS file: /fetish/cu/src/df.c,v
retrieving revision 1.176
diff -p -u -r1.176 df.c
--- src/df.c    22 Aug 2006 03:59:15 -0000      1.176
+++ src/df.c    3 Sep 2006 02:49:29 -0000
@@ -26,7 +26,6 @@
 
 #include "system.h"
 #include "canonicalize.h"
-#include "dirname.h"
 #include "error.h"
 #include "fsusage.h"
 #include "human.h"
Index: src/dircolors.c
===================================================================
RCS file: /fetish/cu/src/dircolors.c,v
retrieving revision 1.100
diff -p -u -r1.100 dircolors.c
--- src/dircolors.c     26 Aug 2006 06:55:58 -0000      1.100
+++ src/dircolors.c     3 Sep 2006 02:49:29 -0000
@@ -24,7 +24,6 @@
 
 #include "system.h"
 #include "dircolors.h"
-#include "dirname.h"
 #include "error.h"
 #include "getline.h"
 #include "obstack.h"
Index: src/dirname.c
===================================================================
RCS file: /fetish/cu/src/dirname.c,v
retrieving revision 1.69
diff -p -u -r1.69 dirname.c
--- src/dirname.c       2 Jun 2005 05:17:24 -0000       1.69
+++ src/dirname.c       3 Sep 2006 02:49:29 -0000
@@ -1,6 +1,6 @@
 /* dirname -- strip suffix from file name
 
-   Copyright (C) 1990-1997, 1999-2002, 2004, 2005 Free Software
+   Copyright (C) 1990-1997, 1999-2002, 2004, 2005, 2006 Free Software
    Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
@@ -27,7 +27,6 @@
 #include "system.h"
 #include "long-options.h"
 #include "error.h"
-#include "dirname.h"
 #include "quote.h"
 
 /* The official name of this program (e.g., no `g' prefix).  */
Index: src/du.c
===================================================================
RCS file: /fetish/cu/src/du.c,v
retrieving revision 1.226
diff -p -u -r1.226 du.c
--- src/du.c    20 May 2006 07:19:53 -0000      1.226
+++ src/du.c    3 Sep 2006 02:49:29 -0000
@@ -31,7 +31,6 @@
 #include <assert.h>
 #include "system.h"
 #include "argmatch.h"
-#include "dirname.h" /* for strip_trailing_slashes */
 #include "error.h"
 #include "exclude.h"
 #include "fprintftime.h"
Index: src/install.c
===================================================================
RCS file: /fetish/cu/src/install.c,v
retrieving revision 1.193
diff -p -u -r1.193 install.c
--- src/install.c       17 Jul 2006 03:09:49 -0000      1.193
+++ src/install.c       3 Sep 2006 02:49:29 -0000
@@ -30,7 +30,6 @@
 #include "error.h"
 #include "cp-hash.h"
 #include "copy.h"
-#include "dirname.h"
 #include "filenamecat.h"
 #include "mkancesdirs.h"
 #include "mkdir-p.h"
Index: src/ln.c
===================================================================
RCS file: /fetish/cu/src/ln.c,v
retrieving revision 1.159
diff -p -u -r1.159 ln.c
--- src/ln.c    28 Aug 2006 23:29:39 -0000      1.159
+++ src/ln.c    3 Sep 2006 02:49:29 -0000
@@ -25,7 +25,6 @@
 #include "system.h"
 #include "same.h"
 #include "backupfile.h"
-#include "dirname.h"
 #include "error.h"
 #include "filenamecat.h"
 #include "quote.h"
Index: src/ls.c
===================================================================
RCS file: /fetish/cu/src/ls.c,v
retrieving revision 1.441
diff -p -u -r1.441 ls.c
--- src/ls.c    28 Aug 2006 23:29:39 -0000      1.441
+++ src/ls.c    3 Sep 2006 02:49:30 -0000
@@ -82,7 +82,6 @@
 #include "acl.h"
 #include "argmatch.h"
 #include "dev-ino.h"
-#include "dirname.h"
 #include "dirfd.h"
 #include "error.h"
 #include "filenamecat.h"
Index: src/mkdir.c
===================================================================
RCS file: /fetish/cu/src/mkdir.c,v
retrieving revision 1.104
diff -p -u -r1.104 mkdir.c
--- src/mkdir.c 17 Jul 2006 03:10:10 -0000      1.104
+++ src/mkdir.c 3 Sep 2006 02:49:30 -0000
@@ -23,7 +23,6 @@
 #include <sys/types.h>
 
 #include "system.h"
-#include "dirname.h"
 #include "error.h"
 #include "lchmod.h"
 #include "mkdir-p.h"
Index: src/mv.c
===================================================================
RCS file: /fetish/cu/src/mv.c,v
retrieving revision 1.174
diff -p -u -r1.174 mv.c
--- src/mv.c    26 Mar 2006 12:07:34 -0000      1.174
+++ src/mv.c    3 Sep 2006 02:49:30 -0000
@@ -28,7 +28,6 @@
 #include "backupfile.h"
 #include "copy.h"
 #include "cp-hash.h"
-#include "dirname.h"
 #include "error.h"
 #include "filenamecat.h"
 #include "quote.h"
Index: src/remove.c
===================================================================
RCS file: /fetish/cu/src/remove.c,v
retrieving revision 1.157
diff -p -u -r1.157 remove.c
--- src/remove.c        27 Aug 2006 19:34:28 -0000      1.157
+++ src/remove.c        3 Sep 2006 02:49:30 -0000
@@ -26,7 +26,6 @@
 #include "system.h"
 #include "cycle-check.h"
 #include "dirfd.h"
-#include "dirname.h"
 #include "error.h"
 #include "euidaccess.h"
 #include "euidaccess-stat.h"
@@ -152,6 +151,43 @@ close_preserve_errno (int fd)
   return result;
 }
 
+/* Like fstatat, but cache the result.  If ST->st_size is -1, the
+   status has not been gotten yet.  If less than -1, fstatat failed
+   with errno == -1 - ST->st_size.  Otherwise, the status has already
+   been gotten, so return 0.  */
+static int
+cache_fstatat (int fd, char const *file, struct stat *st, int flag)
+{
+  if (st->st_size == -1 && fstatat (fd, file, st, flag) != 0)
+    st->st_size = -1 - errno;
+  if (0 <= st->st_size)
+    return 0;
+  errno = -1 - st->st_size;
+  return -1;
+}
+
+/* Initialize a fstatat cache *ST.  */
+static inline void
+cache_stat_init (struct stat *st)
+{
+  st->st_size = -1;
+}
+
+/* Return true if *ST has been statted.  */
+static inline bool
+cache_statted (struct stat *st)
+{
+  return (st->st_size != -1);
+}
+
+/* Return true if *ST has been statted successfully.  */
+static inline bool
+cache_stat_ok (struct stat *st)
+{
+  return (0 <= st->st_size);
+}
+
+
 static void
 hash_freer (void *x)
 {
@@ -661,18 +697,16 @@ is_empty_dir (int fd_cwd, char const *di
 
 /* Return true if FILE is determined to be an unwritable non-symlink.
    Otherwise, return false (including when lstat'ing it fails).
-   If lstat (aka fstatat) succeeds, set *BUF_P to BUF.
+   Set *BUF to the file status.
    This is to avoid calling euidaccess when FILE is a symlink.  */
 static bool
 write_protected_non_symlink (int fd_cwd,
                             char const *file,
                             Dirstack_state const *ds,
-                            struct stat **buf_p,
                             struct stat *buf)
 {
-  if (fstatat (fd_cwd, file, buf, AT_SYMLINK_NOFOLLOW) != 0)
+  if (cache_fstatat (fd_cwd, file, buf, AT_SYMLINK_NOFOLLOW) != 0)
     return false;
-  *buf_p = buf;
   if (S_ISLNK (buf->st_mode))
     return false;
   /* Here, we know FILE is not a symbolic link.  */
@@ -744,41 +778,33 @@ write_protected_non_symlink (int fd_cwd,
    directory FILENAME.  MODE is ignored when FILENAME is not a directory.
    Set *IS_EMPTY to T_YES if FILENAME is an empty directory, and it is
    appropriate to try to remove it with rmdir (e.g. recursive mode).
-   Don't even try to set *IS_EMPTY when MODE == PA_REMOVE_DIR.
-   Set *IS_DIR to T_YES or T_NO if we happen to determine whether
-   FILENAME is a directory.  */
+   Don't even try to set *IS_EMPTY when MODE == PA_REMOVE_DIR.  */
 static enum RM_status
 prompt (int fd_cwd, Dirstack_state const *ds, char const *filename,
+       struct stat *sbuf,
        struct rm_options const *x, enum Prompt_action mode,
-       Ternary *is_dir, Ternary *is_empty)
+       Ternary *is_empty)
 {
   bool write_protected = false;
-  struct stat *sbuf = NULL;
-  struct stat buf;
 
   *is_empty = T_UNKNOWN;
-  *is_dir = T_UNKNOWN;
 
   if (((!x->ignore_missing_files & (x->interactive | x->stdin_tty))
        && (write_protected = write_protected_non_symlink (fd_cwd, filename,
-                                                         ds, &sbuf, &buf)))
+                                                         ds, sbuf)))
       || x->interactive)
     {
-      if (sbuf == NULL)
+      if (cache_fstatat (fd_cwd, filename, sbuf, AT_SYMLINK_NOFOLLOW) != 0)
        {
-         sbuf = &buf;
-         if (fstatat (fd_cwd, filename, sbuf, AT_SYMLINK_NOFOLLOW))
-           {
-             /* lstat failed.  This happens e.g., with `rm '''.  */
-             error (0, errno, _("cannot remove %s"),
-                    quote (full_filename (filename)));
-             return RM_ERROR;
-           }
+         /* This happens, e.g., with `rm '''.  */
+         error (0, errno, _("cannot remove %s"),
+                quote (full_filename (filename)));
+         return RM_ERROR;
        }
 
       if (S_ISDIR (sbuf->st_mode) && !x->recursive)
        {
-         error (0, EISDIR, _("cannot remove directory %s"),
+         error (0, EISDIR, _("cannot remove %s"),
                 quote (full_filename (filename)));
          return RM_ERROR;
        }
@@ -795,8 +821,6 @@ prompt (int fd_cwd, Dirstack_state const
       {
        char const *quoted_name = quote (full_filename (filename));
 
-       *is_dir = (S_ISDIR (sbuf->st_mode) ? T_YES : T_NO);
-
        /* FIXME: use a variant of error (instead of fprintf) that doesn't
           append a newline.  Then we won't have to declare program_name in
           this file.  */
@@ -832,13 +856,15 @@ prompt (int fd_cwd, Dirstack_state const
 
 /* Return true if FILENAME is a directory (and not a symlink to a directory).
    Otherwise, including the case in which lstat fails, return false.
+   *ST is FILENAME's tstatus.
    Do not modify errno.  */
 static inline bool
-is_dir_lstat (char const *filename)
+is_dir_lstat (char const *filename, struct stat *st)
 {
-  struct stat sbuf;
   int saved_errno = errno;
-  bool is_dir = lstat (filename, &sbuf) == 0 && S_ISDIR (sbuf.st_mode);
+  bool is_dir =
+    (cache_fstatat (AT_FDCWD, filename, st, AT_SYMLINK_NOFOLLOW) == 0
+     && S_ISDIR (st->st_mode));
   errno = saved_errno;
   return is_dir;
 }
@@ -896,12 +922,13 @@ is_dir_lstat (char const *filename)
 
 static enum RM_status
 remove_entry (int fd_cwd, Dirstack_state const *ds, char const *filename,
+             struct stat *st,
              struct rm_options const *x, struct dirent const *dp)
 {
-  Ternary is_dir;
   Ternary is_empty_directory;
-  enum RM_status s = prompt (fd_cwd, ds, filename, x, PA_DESCEND_INTO_DIR,
-                            &is_dir, &is_empty_directory);
+  enum RM_status s = prompt (fd_cwd, ds, filename, st, x, PA_DESCEND_INTO_DIR,
+                            &is_empty_directory);
+  bool known_to_be_dir = (cache_stat_ok (st) && S_ISDIR (st->st_mode));
 
   if (s != RM_OK)
     return s;
@@ -922,9 +949,9 @@ remove_entry (int fd_cwd, Dirstack_state
 
   if (cannot_unlink_dir ())
     {
-      if (is_dir == T_YES && ! x->recursive)
+      if (known_to_be_dir && ! x->recursive)
        {
-         error (0, EISDIR, _("cannot remove directory %s"),
+         error (0, EISDIR, _("cannot remove %s"),
                 quote (full_filename (filename)));
          return RM_ERROR;
        }
@@ -941,7 +968,7 @@ remove_entry (int fd_cwd, Dirstack_state
         unlink call.  If FILENAME is a command-line argument, then dp is NULL,
         so we'll first try to unlink it.  Using unlink here is ok, because it
         cannot remove a directory.  */
-      if ((dp && DT_MUST_BE (dp, DT_DIR)) || is_dir == T_YES)
+      if ((dp && DT_MUST_BE (dp, DT_DIR)) || known_to_be_dir)
        return RM_NONEMPTY_DIR;
 
       DO_UNLINK (fd_cwd, filename, x);
@@ -949,7 +976,7 @@ remove_entry (int fd_cwd, Dirstack_state
       /* Upon a failed attempt to unlink a directory, most non-Linux systems
         set errno to the POSIX-required value EPERM.  In that case, change
         errno to EISDIR so that we emit a better diagnostic.  */
-      if (! x->recursive && errno == EPERM && is_dir_lstat (filename))
+      if (! x->recursive && errno == EPERM && is_dir_lstat (filename, st))
        errno = EISDIR;
 
       if (! x->recursive
@@ -965,16 +992,20 @@ remove_entry (int fd_cwd, Dirstack_state
     }
   else
     {
-      /* If we don't already know whether FILENAME is a directory, find out 
now.
-        Then, if it's a non-directory, we can use unlink on it.  */
-      if (is_dir == T_UNKNOWN)
+      /* If we don't already know whether FILENAME is a directory,
+        find out now.  Then, if it's a non-directory, we can use
+        unlink on it.  */
+      bool is_dir;
+
+      if (cache_statted (st))
+       is_dir = known_to_be_dir;
+      else
        {
          if (dp && DT_IS_KNOWN (dp))
-           is_dir = DT_MUST_BE (dp, DT_DIR) ? T_YES : T_NO;
+           is_dir = DT_MUST_BE (dp, DT_DIR);
          else
            {
-             struct stat sbuf;
-             if (fstatat (fd_cwd, filename, &sbuf, AT_SYMLINK_NOFOLLOW))
+             if (fstatat (fd_cwd, filename, st, AT_SYMLINK_NOFOLLOW))
                {
                  if (errno == ENOENT && x->ignore_missing_files)
                    return RM_OK;
@@ -984,11 +1015,11 @@ remove_entry (int fd_cwd, Dirstack_state
                  return RM_ERROR;
                }
 
-             is_dir = S_ISDIR (sbuf.st_mode) ? T_YES : T_NO;
+             is_dir = !! S_ISDIR (st->st_mode);
            }
        }
 
-      if (is_dir == T_NO)
+      if (! is_dir)
        {
          /* At this point, barring race conditions, FILENAME is known
             to be a non-directory, so it's ok to try to unlink it.  */
@@ -1002,7 +1033,7 @@ remove_entry (int fd_cwd, Dirstack_state
 
       if (! x->recursive)
        {
-         error (0, EISDIR, _("cannot remove directory %s"),
+         error (0, EISDIR, _("cannot remove %s"),
                 quote (full_filename (filename)));
          return RM_ERROR;
        }
@@ -1130,7 +1161,8 @@ remove_cwd_entries (DIR **dirp,
         case can decide whether to use unlink or chdir.
         Systems without the d_type member will have to endure
         the performance hit of first calling lstat F. */
-      tmp_status = remove_entry (dirfd (*dirp), ds, f, x, dp);
+      cache_stat_init (subdir_sb);
+      tmp_status = remove_entry (dirfd (*dirp), ds, f, subdir_sb, x, dp);
       switch (tmp_status)
        {
        case RM_OK:
@@ -1224,10 +1256,10 @@ remove_cwd_entries (DIR **dirp,
 
 static enum RM_status
 remove_dir (int fd_cwd, Dirstack_state *ds, char const *dir,
+           struct stat *dir_st,
            struct rm_options const *x, int *cwd_errno)
 {
   enum RM_status status;
-  struct stat dir_sb;
 
   /* There is a race condition in that an attacker could replace the nonempty
      directory, DIR, with a symlink between the preceding call to rmdir
@@ -1237,7 +1269,7 @@ remove_dir (int fd_cwd, Dirstack_state *
      fd_to_subdirp's fstat, along with the `fstat' and the dev/ino
      comparison in AD_push ensure that we detect it and fail.  */
 
-  DIR *dirp = fd_to_subdirp (fd_cwd, dir, x, 0, &dir_sb, ds, cwd_errno);
+  DIR *dirp = fd_to_subdirp (fd_cwd, dir, x, 0, dir_st, ds, cwd_errno);
 
   if (dirp == NULL)
     {
@@ -1262,14 +1294,14 @@ remove_dir (int fd_cwd, Dirstack_state *
       return RM_ERROR;
     }
 
-  if (ROOT_DEV_INO_CHECK (x->root_dev_ino, &dir_sb))
+  if (ROOT_DEV_INO_CHECK (x->root_dev_ino, dir_st))
     {
       ROOT_DEV_INO_WARN (full_filename (dir));
       status = RM_ERROR;
       goto closedir_and_return;
     }
 
-  AD_push (dirfd (dirp), ds, dir, &dir_sb);
+  AD_push (dirfd (dirp), ds, dir, dir_st);
   AD_INIT_OTHER_MEMBERS ();
 
   status = RM_OK;
@@ -1314,10 +1346,10 @@ remove_dir (int fd_cwd, Dirstack_state *
               prompts the user.  E.g., we already know that D is a directory
               and that it's almost certainly empty, yet we lstat it.
               But that's no big deal since we're interactive.  */
-           Ternary is_dir;
+           struct stat empty_st;  cache_stat_init (&empty_st);
            Ternary is_empty;
-           enum RM_status s = prompt (fd, ds, empty_dir, x,
-                                      PA_REMOVE_DIR, &is_dir, &is_empty);
+           enum RM_status s = prompt (fd, ds, empty_dir, &empty_st, x,
+                                      PA_REMOVE_DIR, &is_empty);
 
            if (s != RM_OK)
              {
@@ -1371,18 +1403,39 @@ static enum RM_status
 rm_1 (Dirstack_state *ds, char const *filename,
       struct rm_options const *x, int *cwd_errno)
 {
+  struct stat st;
+  cache_stat_init (&st);
+
   char const *base = last_component (filename);
   if (dot_or_dotdot (base))
     {
-      error (0, 0, _("cannot remove `.' or `..'"));
+      error (0, 0, _(base == filename
+                    ? "cannot remove directory %s"
+                    : "cannot remove %s directory %s"),
+            quote_n (0, base), quote_n (1, filename));
       return RM_ERROR;
     }
+  if (x->root_dev_ino)
+    {
+      if (cache_fstatat (AT_FDCWD, filename, &st, AT_SYMLINK_NOFOLLOW) != 0)
+       {
+         if (errno == ENOENT && x->ignore_missing_files)
+           return RM_OK;
+         error (0, errno, _("cannot remove %s"), quote (filename));
+         return RM_ERROR;
+       }
+      if (SAME_INODE (st, *(x->root_dev_ino)))
+       {
+         error (0, 0, _("cannot remove root directory %s"), quote (filename));
+         return RM_ERROR;
+       }
+    }
 
   AD_push_initial (ds);
   AD_INIT_OTHER_MEMBERS ();
 
   int fd_cwd = AT_FDCWD;
-  enum RM_status status = remove_entry (fd_cwd, ds, filename, x, NULL);
+  enum RM_status status = remove_entry (fd_cwd, ds, filename, &st, x, NULL);
   if (status == RM_NONEMPTY_DIR)
     {
       /* In the event that remove_dir->remove_cwd_entries detects
@@ -1391,7 +1444,7 @@ rm_1 (Dirstack_state *ds, char const *fi
       if (setjmp (ds->current_arg_jumpbuf))
        status = RM_ERROR;
       else
-       status = remove_dir (fd_cwd, ds, filename, x, cwd_errno);
+       status = remove_dir (fd_cwd, ds, filename, &st, x, cwd_errno);
 
       AD_stack_clear (ds);
     }
Index: src/remove.h
===================================================================
RCS file: /fetish/cu/src/remove.h,v
retrieving revision 1.15
diff -p -u -r1.15 remove.h
--- src/remove.h        2 Nov 2005 21:51:57 -0000       1.15
+++ src/remove.h        3 Sep 2006 02:49:30 -0000
@@ -33,8 +33,8 @@ struct rm_options
   /* If true, recursively remove directories.  */
   bool recursive;
 
-  /* Pointer to the device and inode numbers of `/', when --recursive.
-     Otherwise NULL.  */
+  /* Pointer to the device and inode numbers of `/', when --recursive
+     and preserving `/'.  Otherwise NULL.  */
   struct dev_ino *root_dev_ino;
 
   /* If nonzero, stdin is a tty.  */
Index: src/rm.c
===================================================================
RCS file: /fetish/cu/src/rm.c,v
retrieving revision 1.139
diff -p -u -r1.139 rm.c
--- src/rm.c    20 Feb 2006 12:48:11 -0000      1.139
+++ src/rm.c    3 Sep 2006 02:49:30 -0000
@@ -50,7 +50,6 @@
 
 #include "system.h"
 #include "argmatch.h"
-#include "dirname.h"
 #include "error.h"
 #include "lstat.h"
 #include "quote.h"
@@ -171,8 +170,8 @@ Remove (unlink) the FILE(s).\n\
                           always (-i).  Without WHEN, prompt always\n\
 "), stdout);
       fputs (_("\
-      --no-preserve-root  do not treat `/' specially (the default)\n\
-      --preserve-root   fail to operate recursively on `/'\n\
+      --no-preserve-root  do not treat `/' specially\n\
+      --preserve-root   do not remove `/' (default)\n\
   -r, -R, --recursive   remove directories and their contents recursively\n\
   -v, --verbose         explain what is being done\n\
 "), stdout);
@@ -221,7 +220,7 @@ rm_option_init (struct rm_options *x)
 int
 main (int argc, char **argv)
 {
-  bool preserve_root = false;
+  bool preserve_root = true;
   struct rm_options x;
   bool prompt_once = false;
   int c;
Index: src/rmdir.c
===================================================================
RCS file: /fetish/cu/src/rmdir.c,v
retrieving revision 1.83
diff -p -u -r1.83 rmdir.c
--- src/rmdir.c 22 Sep 2005 06:56:21 -0000      1.83
+++ src/rmdir.c 3 Sep 2006 02:49:30 -0000
@@ -1,5 +1,7 @@
 /* rmdir -- remove directories
-   Copyright (C) 90, 91, 1995-2002, 2004, 2005 Free Software Foundation, Inc.
+
+   Copyright (C) 90, 91, 1995-2002, 2004, 2005, 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
@@ -28,7 +30,6 @@
 #include <sys/types.h>
 
 #include "system.h"
-#include "dirname.h"
 #include "error.h"
 #include "quotearg.h"
 
Index: src/shred.c
===================================================================
RCS file: /fetish/cu/src/shred.c,v
retrieving revision 1.129
diff -p -u -r1.129 shred.c
--- src/shred.c 26 Aug 2006 06:55:58 -0000      1.129
+++ src/shred.c 3 Sep 2006 02:49:30 -0000
@@ -95,7 +95,6 @@
 
 #include "system.h"
 #include "xstrtol.h"
-#include "dirname.h"
 #include "error.h"
 #include "fcntl--.h"
 #include "getpagesize.h"
Index: src/split.c
===================================================================
RCS file: /fetish/cu/src/split.c,v
retrieving revision 1.113
diff -p -u -r1.113 split.c
--- src/split.c 26 Mar 2006 12:08:10 -0000      1.113
+++ src/split.c 3 Sep 2006 02:49:30 -0000
@@ -28,7 +28,6 @@
 #include <sys/types.h>
 
 #include "system.h"
-#include "dirname.h"
 #include "error.h"
 #include "fd-reopen.h"
 #include "fcntl--.h"
Index: src/su.c
===================================================================
RCS file: /fetish/cu/src/su.c,v
retrieving revision 1.92
diff -p -u -r1.92 su.c
--- src/su.c    21 Aug 2006 07:30:47 -0000      1.92
+++ src/su.c    3 Sep 2006 02:49:30 -0000
@@ -60,7 +60,6 @@
 #define getusershell _getusershell_sys_proto_
 
 #include "system.h"
-#include "dirname.h"
 #include "getpass.h"
 
 #undef getusershell
Index: src/system.h
===================================================================
RCS file: /fetish/cu/src/system.h,v
retrieving revision 1.159
diff -p -u -r1.159 system.h
--- src/system.h        29 Aug 2006 14:26:52 -0000      1.159
+++ src/system.h        3 Sep 2006 02:49:30 -0000
@@ -368,17 +368,21 @@ uid_t getuid ();
 #define X2REALLOC(P, PN) ((void) verify_true (sizeof *(P) == 1), \
                           x2realloc (P, PN))
 
-/* Include automatically-generated macros for unlocked I/O.  */
 #include "unlocked-io.h"
 #include "same-inode.h"
 
+#include "dirname.h"
+
 static inline bool
 dot_or_dotdot (char const *file_name)
 {
-  return (file_name[0] == '.'
-         && (file_name[1] == '\0'
-             || (file_name[1] == '.'
-                 && file_name[2] == '\0')));
+  if (file_name[0] == '.')
+    {
+      char sep = file_name[(file_name[1] == '.') + 1];
+      return (! sep || ISSLASH (sep));
+    }
+  else
+    return false;
 }
 
 /* A wrapper for readdir so that callers don't see entries for `.' or `..'.  */
Index: tests/rm/Makefile.am
===================================================================
RCS file: /fetish/cu/tests/rm/Makefile.am,v
retrieving revision 1.35
diff -p -u -r1.35 Makefile.am
--- tests/rm/Makefile.am        17 Aug 2006 19:58:36 -0000      1.35
+++ tests/rm/Makefile.am        3 Sep 2006 02:49:30 -0000
@@ -31,7 +31,7 @@ TESTS = \
   fail-2eperm \
   cycle i-no-r fail-eperm \
   dangling-symlink rm1 rm2 rm3 rm4 rm5 \
-  unread2 r-1 r-2 r-3 i-1 ir-1 f-1 sunos-1 deep-1 hash \
+  unread2 r-1 r-2 r-3 r-4 i-1 ir-1 f-1 sunos-1 deep-1 hash \
   interactive-always interactive-once \
   isatty # unreadable empty-name
 EXTRA_DIST = $(TESTS)
--- /dev/null   2005-09-24 22:00:15.000000000 -0700
+++ tests/rm/r-4        2006-09-02 18:25:37.000000000 -0700
@@ -0,0 +1,56 @@
+#!/bin/sh
+# Try to remove '.' and '..' recursively.
+
+# 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
+  rm --version
+  set -x
+fi
+
+pwd=`pwd`
+t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp=$t0/$$
+trap 'status=$?; cd $pwd; 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 a || framework_failure=1
+cd $pwd || framework_failure=1
+
+if test $framework_failure = 1; then
+  echo 'failure in testing framework'
+  (exit 1); exit 1
+fi
+
+fail=0
+rm -fr $tmp/. 2>/dev/null && fail=1
+rm -fr $tmp/./ 2>/dev/null && fail=1
+rm -fr $tmp/.//// 2>/dev/null && fail=1
+rm -fr $tmp/.. 2>/dev/null && fail=1
+rm -fr $tmp/../ 2>/dev/null && fail=1
+
+# This test is too dangerous -- if there's a bug you're wiped out!
+# rm -fr / 2>/dev/null && fail=1
+
+test -f $tmp/a || fail=1
+
+(exit $fail); exit $fail





reply via email to

[Prev in Thread] Current Thread [Next in Thread]