bug-gnulib
[Top][All Lists]
Advanced

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

Re: rm 8.1


From: Jim Meyering
Subject: Re: rm 8.1
Date: Tue, 01 Dec 2009 12:14:35 +0100

Ladislav Hagara wrote:
> the behaviour of rm from last stable coreutils 8.1 is quite different
> from previous 7.6 one.
....
> Is this a bug or a new feature of rm 8.1?

Thank you very much for the report!
That is most definitely a bug.

The fix has two parts:

in gnulib:

    The fts_open function in gnulib's fts module must not fail
    immediately upon spotting an empty string argument.
    That is the sole functional change.  The rest is clean-up:

in coreutils:

    rm: fix empty-name bug introduced with conversion to use fts

    While "rm ''" would properly fail, "rm F1 '' F2" would fail
    to remove F1 and F2, due to the empty string argument.
    This bug was introduced on 2009-07-12, via commit 4f73ecaf,
    "rm: rewrite to use fts".
    * NEWS (Bug fixes): Describe it.
    * tests/rm/empty-name: Adjust for changed diagnostic.
    (mk_file): Define, copied from misc/ls-misc.
    (empty-name-2): New test, for today's fix.
    * lib/xfts.c (xfts_open): Reflect the change in fts_open, now that
    it no longer fails immediately when one argument is the empty string.
    Assert that the bit flags were not the cause of failure.
    * po/POTFILES.in: Remove xfts.c.
    * THANKS: Update.
    Reported by Ladislav Hagara.

I'll push something like the following today,
once I've audited fts.c more closely to ensure
that lifting the prohibition on empty names is ok.


>From 3783b3e6a4166212b4df5d958d04493e341c3b90 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 1 Dec 2009 12:06:34 +0100
Subject: [PATCH] fts: fts_open: do not let an empty string cause immediate 
failure

This is required in support of GNU rm, for which the command
"rm A '' B" must process and remove both A and B, in spite of
the empty string argument.
* lib/fts.c (fts_open): Do not let the presence of an empty string
cause fts_open to fail immediately.  Most fts-using tools must be
able to process all arguments, in order, and can be expected to
diagnose such arguments themselves.
---
 ChangeLog |   11 +++++++++++
 lib/fts.c |    7 ++-----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 07918de..6eec830 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2009-12-01  Jim Meyering  <address@hidden>
+
+       fts: fts_open: do not let an empty string cause immediate failure
+       This is required in support of GNU rm, for which the command
+       "rm A '' B" must process and remove both A and B, in spite of
+       the empty string argument.
+       * lib/fts.c (fts_open): Do not let the presence of an empty string
+       cause fts_open to fail immediately.  Most fts-using tools must be
+       able to process all arguments, in order, and can be expected to
+       diagnose such arguments themselves.
+
 2009-11-30  Eric Blake  <address@hidden>

        utimens: fix compilation error
diff --git a/lib/fts.c b/lib/fts.c
index 21c6658..a3b19eb 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -474,11 +474,8 @@ fts_open (char * const *argv,

        /* Allocate/initialize root(s). */
        for (root = NULL, nitems = 0; *argv != NULL; ++argv, ++nitems) {
-               /* Don't allow zero-length file names. */
-               if ((len = strlen(*argv)) == 0) {
-                       __set_errno (ENOENT);
-                       goto mem3;
-               }
+               /* *Do* allow zero-length file names. */
+               len = strlen(*argv);

                if ((p = fts_alloc(sp, *argv, len)) == NULL)
                        goto mem3;
--
1.6.6.rc0.324.gb5bf2


>From b1b663c6f859e2c5008417101d16fcd57876e8ac Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 1 Dec 2009 12:02:11 +0100
Subject: [PATCH] rm: fix empty-name bug introduced with conversion to use fts

While "rm ''" would properly fail, "rm F1 '' F2" would fail
to remove F1 and F2, due to the empty string argument.
This bug was introduced on 2009-07-12, via commit 4f73ecaf,
"rm: rewrite to use fts".
* NEWS (Bug fixes): Describe it.
* tests/rm/empty-name: Adjust for changed diagnostic.
(mk_file): Define, copied from misc/ls-misc.
(empty-name-2): New test, for today's fix.
* lib/xfts.c (xfts_open): Reflect the change in fts_open, now that
it no longer fails immediately when one argument is the empty string.
Assert that the bit flags were not the cause of failure.
* po/POTFILES.in: Remove xfts.c.
* THANKS: Update.
Reported by Ladislav Hagara.
---
 NEWS                |    8 ++++++++
 THANKS              |    1 +
 lib/xfts.c          |   29 ++++++-----------------------
 po/POTFILES.in      |    1 -
 tests/rm/empty-name |   18 +++++++++++++++++-
 5 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/NEWS b/NEWS
index df258e2..9f7bf2e 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,14 @@ GNU coreutils NEWS                                    -*- 
outline -*-

 * Noteworthy changes in release ?.? (????-??-??) [?]

+** Bug fixes
+
+  rm once again handles zero-length arguments properly.
+  The rewrite to make rm use fts introduced a regression whereby
+  a command like "rm a '' b" would fail to remove "a" and "b", due to
+  the presence of the empty string argument.
+  [bug introduced in coreutils-8.0]
+

 * Noteworthy changes in release 8.1 (2009-11-18) [stable]

diff --git a/THANKS b/THANKS
index f9bc476..52d4170 100644
--- a/THANKS
+++ b/THANKS
@@ -334,6 +334,7 @@ Kirk Kelsey                         address@hidden
 Kristin E Thomas                    address@hidden
 Kjetil Torgrim Homme                address@hidden
 Kristoffer Rose                     address@hidden
+Ladislav Hagara                     address@hidden
 Larry McVoy                         address@hidden
 Lars Hecking                        address@hidden
 Leah Q                              address@hidden
diff --git a/lib/xfts.c b/lib/xfts.c
index 9c46f6a..dd86a5c 100644
--- a/lib/xfts.c
+++ b/lib/xfts.c
@@ -21,13 +21,9 @@

 #include <stdbool.h>
 #include <stdlib.h>
+#include <errno.h>
+#include <assert.h>

-#include "error.h"
-
-#include "gettext.h"
-#define _(msgid) gettext (msgid)
-
-#include "quote.h"
 #include "xalloc.h"
 #include "xfts.h"

@@ -40,23 +36,10 @@ xfts_open (char * const *argv, int options,
   FTS *fts = fts_open (argv, options | FTS_CWDFD, compar);
   if (fts == NULL)
     {
-      /* This can fail in three ways: out of memory, invalid bit_flags,
-         and one or more of the FILES is an empty string.  We could try
-         to decipher that errno==EINVAL means invalid bit_flags and
-         errno==ENOENT means there's an empty string, but that seems wrong.
-         Ideally, fts_open would return a proper error indicator.  For now,
-         we'll presume that the bit_flags are valid and just check for
-         empty strings.  */
-      bool invalid_arg = false;
-      for (; *argv; ++argv)
-        {
-          if (**argv == '\0')
-            invalid_arg = true;
-        }
-      if (invalid_arg)
-        error (EXIT_FAILURE, 0, _("invalid argument: %s"), quote (""));
-      else
-        xalloc_die ();
+      /* This can fail in two ways: out of memory or with errno==EINVAL,
+         which indicates it was called with invalid bit_flags.  */
+      assert (errno != EINVAL);
+      xalloc_die ();
     }

   return fts;
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 9a46a9a..e744dda 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -28,7 +28,6 @@ lib/verror.c
 lib/version-etc.c
 lib/xalloc-die.c
 lib/xfreopen.c
-lib/xfts.c
 lib/xmemcoll.c
 lib/xmemxfrm.c
 lib/xprintf.c
diff --git a/tests/rm/empty-name b/tests/rm/empty-name
index 08cc8e3..27f76d0 100755
--- a/tests/rm/empty-name
+++ b/tests/rm/empty-name
@@ -29,12 +29,28 @@ use strict;

 my $prog = 'rm';

+# FIXME: copied from misc/ls-misc; factor into Coreutils.pm?
+sub mk_file(@)
+{
+  foreach my $f (@_)
+    {
+      open (F, '>', $f) && close F
+        or die "creating $f: $!\n";
+    }
+}
+
 my @Tests =
     (
      # test-name options input expected-output
      #
      ['empty-name-1', "''", {EXIT => 1},
-      {ERR => "$prog: invalid argument: `'\n"}],
+      {ERR => "$prog: cannot remove `': No such file or directory\n"}],
+
+     ['empty-name-2', "a '' b", {EXIT => 1},
+      {ERR => "$prog: cannot remove `': No such file or directory\n"},
+      {PRE => sub { mk_file qw(a b) }},
+      {POST => sub {-f 'a' || -f 'b' and die "a or b remain\n" }},
+     ],
     );

 my $save_temps = $ENV{SAVE_TEMPS};
--
1.6.6.rc0.324.gb5bf2




reply via email to

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