bug-coreutils
[Top][All Lists]
Advanced

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

bug fix: mv and 'cp -r' no longer fail when ...


From: Jim Meyering
Subject: bug fix: mv and 'cp -r' no longer fail when ...
Date: Fri, 08 Sep 2006 19:23:55 +0200

I hesitated to remove the tests that provoked this sort of error:

    $ rm -rf a c; mkdir a; mv a c/
    mv: target `c/' is not a directory: No such file or directory
    [Exit 1]

They were added to prevent a command like this from succeeding:

    $ touch a b; mv a b/

But on modern systems, the mv fails even without the now-removed tests,
because the `rename ("a", "b/")' syscall fails.  The argument was that
some systems (e.g., older Solaris) would succeed, in spite of the
trailing slash.  They simply ignored any trailing slashes.

If anyone finds that the use of GNU mv in the example above
now mistakenly succeeds on some system, please report the
problem, and we'll think about whether to write another rename
wrapper to compensate.

The same argument applies to `cp -r', but I've studied it less.
The only remaining tool with a target_directory_operand function...
was going to say "is ln", but obviously "install" is affected too.
The difference with ln (where I've left the target_directory_operand
function unchanged) is that there is no meaningful way to invoke
"ln ... DIR D2/".  So there's no reason to change it.

I'll deal with install separately -- probably just like cp and mv.


2006-09-08  Jim Meyering  <address@hidden>

        mv and "cp -r" no longer fail when invoked with two arguments
        where the first one names a directory and the second name ends in
        a slash and doesn't exist.  E.g., "mv dir B/", for nonexistent B,
        now succeeds, once more. This reverts part of the 2004-06-27
        change for 5.3.0.
        * NEWS: Say the above.
        * src/mv.c (target_directory_operand): Don't require (here)
        that the target operand "look like" a directory.  This change
        pushes the test down to the rename syscall level, where a
        "mv dir existing-non-dir/" will mistakenly succeed on older systems
        that ignore trailing slashes in the rename destination argument.
        * src/cp.c (target_directory_operand): Likewise, but for cp.
        * tests/mv/trailing-slash: Exercise the above fixes.
        * tests/cp/trailing-slash: New file.
        * tests/cp/Makefile.am (EXTRA_DIST): Add trailing-slash.

Index: NEWS
===================================================================
RCS file: /fetish/cu/NEWS,v
retrieving revision 1.416
diff -u -r1.416 NEWS
--- NEWS        4 Sep 2006 07:50:16 -0000       1.416
+++ NEWS        8 Sep 2006 16:46:35 -0000
@@ -27,6 +27,11 @@
   "mv -T --verbose --backup=t A B" now prints the " (backup: B.~1~)"
   suffix when A and B are directories as well as when they are not.
 
+  mv and "cp -r" no longer fail when invoked with two arguments
+  where the first one names a directory and the second name ends in
+  a slash and doesn't exist.  E.g., "mv dir B/", for nonexistent B,
+  now succeeds, once more.  This bug was introduced in coreutils-5.3.0.
+
 
 * Major changes in release 6.1 (2006-08-19) [unstable]
 
Index: src/mv.c
===================================================================
RCS file: /fetish/cu/src/mv.c,v
retrieving revision 1.175
diff -u -r1.175 mv.c
--- src/mv.c    3 Sep 2006 02:53:16 -0000       1.175
+++ src/mv.c    8 Sep 2006 11:47:34 -0000
@@ -147,16 +147,11 @@
 static bool
 target_directory_operand (char const *file)
 {
-  char const *b = last_component (file);
-  size_t blen = strlen (b);
-  bool looks_like_a_dir = (blen == 0 || ISSLASH (b[blen - 1]));
   struct stat st;
   int err = (stat (file, &st) == 0 ? 0 : errno);
   bool is_a_dir = !err && S_ISDIR (st.st_mode);
   if (err && err != ENOENT)
     error (EXIT_FAILURE, err, _("accessing %s"), quote (file));
-  if (is_a_dir < looks_like_a_dir)
-    error (EXIT_FAILURE, err, _("target %s is not a directory"), quote (file));
   return is_a_dir;
 }
 
@@ -258,7 +253,6 @@
      function that ignores a trailing slash.  I believe the Linux
      rename semantics are POSIX and susv2 compliant.  */
 
-  strip_trailing_slashes (dest);
   if (remove_trailing_slashes)
     strip_trailing_slashes (source);
 
Index: src/cp.c
===================================================================
RCS file: /fetish/cu/src/cp.c,v
retrieving revision 1.223
diff -u -r1.223 cp.c
--- src/cp.c    3 Sep 2006 02:53:16 -0000       1.223
+++ src/cp.c    8 Sep 2006 14:23:10 -0000
@@ -518,9 +518,6 @@
 static bool
 target_directory_operand (char const *file, struct stat *st, bool *new_dst)
 {
-  char const *b = last_component (file);
-  size_t blen = strlen (b);
-  bool looks_like_a_dir = (blen == 0 || ISSLASH (b[blen - 1]));
   int err = (stat (file, st) == 0 ? 0 : errno);
   bool is_a_dir = !err && S_ISDIR (st->st_mode);
   if (err)
@@ -529,8 +526,6 @@
        error (EXIT_FAILURE, err, _("accessing %s"), quote (file));
       *new_dst = true;
     }
-  if (is_a_dir < looks_like_a_dir)
-    error (EXIT_FAILURE, err, _("target %s is not a directory"), quote (file));
   return is_a_dir;
 }
 
Index: tests/mv/trailing-slash
===================================================================
RCS file: /fetish/cu/tests/mv/trailing-slash,v
retrieving revision 1.3
diff -u -r1.3 trailing-slash
--- tests/mv/trailing-slash     17 Aug 2006 19:58:35 -0000      1.3
+++ tests/mv/trailing-slash     8 Sep 2006 16:15:54 -0000
@@ -1,8 +1,10 @@
 #!/bin/sh
 # On some operating systems, e.g. SunOS-4.1.1_U1 on sun3x,
 # rename() doesn't accept trailing slashes.
+# Also, ensure that "mv dir non-exist-dir/" works.
+# Also, ensure that "cp dir non-exist-dir/" works.
 
-# Copyright (C) 2004 Free Software Foundation, Inc.
+# Copyright (C) 2004, 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
@@ -37,7 +39,7 @@
 mkdir foo || framework_failure=1
 
 if test $framework_failure = 1; then
-  echo 'failure in testing framework'
+  echo 'failure in testing framework' 1>&2
   exit 1
 fi
 
@@ -45,4 +47,31 @@
 
 mv foo/ bar || fail=1
 
+# mv and cp would misbehave for coreutils versions [5.3.0..5.97], 6.0 and 6.1
+for cmd in mv 'cp -r'; do
+  for opt in '' -T -u; do
+    rm -rf d e || framework_failure=1
+    mkdir d    || framework_failure=1
+    if test $framework_failure = 1; then
+      echo 'failure in testing framework'
+      (exit 1); exit 1
+    fi
+
+    $cmd $opt d e/ || fail=1
+    if test "$cmd" = mv; then
+      test -d d && fail=1
+    else
+      test -d d || fail=1
+    fi
+    test -d e || fail=1
+  done
+done
+
+# We would like the erroneous-looking "mv any non-dir/" to fail,
+# but with the current implementation, it depends on how the
+# underlying rename syscall handles the trailing slash.
+# It does fail, as desired, on recent Linux and Solaris systems.
+#touch a a2
+#mv a a2/ && fail=1
+
 (exit $fail); exit $fail
Index: tests/cp/trailing-slash
===================================================================
RCS file: tests/cp/trailing-slash
diff -N tests/cp/trailing-slash
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/cp/trailing-slash     8 Sep 2006 15:33:58 -0000
@@ -0,0 +1,2 @@
+# this is just a place-holder.
+# For trailing-slash-related tests, see ../mv/trailing-slash.
Index: tests/cp/Makefile.am
===================================================================
RCS file: /fetish/cu/tests/cp/Makefile.am,v
retrieving revision 1.37
diff -u -r1.37 Makefile.am
--- tests/cp/Makefile.am        5 Sep 2006 11:50:57 -0000       1.37
+++ tests/cp/Makefile.am        8 Sep 2006 15:34:13 -0000
@@ -31,7 +31,7 @@
   same-file cp-mv-backup symlink-slash slink-2-slink fail-perm dir-slash \
   perm cp-HL special-bits link dir-rm-dest cp-parents deref-slink \
   dir-vs-file into-self
-EXTRA_DIST = $(TESTS)
+EXTRA_DIST = $(TESTS) trailing-slash
 TESTS_ENVIRONMENT = \
   MAKE=$(MAKE) \
   CONFIG_HEADER=$(CONFIG_HEADER) \




reply via email to

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