Re: mv fails to move files

[Jim Meyering]

Jim Meyering <address@hidden> wrote:
> Iida Yosiaki <address@hidden> wrote:
>> It seems for me that mv in coreutils 4.5.8 failes to move files
>> in a certain condition.
>>
>> * How to reproduce the bug.
> ...
>
> Thank you for the very nice bug report!
>
> To reassure anyone who might be worrying about whether
> this is serious, note that this happens only when hard-linked
> files are moved (or copied with cp --preserve=link) onto existing
> destination files without using a --backup option.
>
> Your test actually exposed two problems:
>
>   * how does mv (and cp --preserve=link) handle existing destination
>     files that end up being the target of a link call?
>
>   * how is the dev/ino<->filename mapping managed when a file with
>     more than one hard link cannot be copied/moved
>
> Here's how I've fixed the first one.
...

And here's the fix for the second part:

        * src/copy.c (copy_internal) [un_backup]: When recovering from a
        failure to create a hard link, do not remove the entry associating
        the source dev/ino with the destination file name.
        * tests/mv/Makefile.am (TESTS): Add hard-3.
        * tests/mv/hard-3: New test, for the above-fixed bug.
        Inspired by a report from Iida Yosiaki.

Index: src/copy.c
===================================================================
RCS file: /fetish/cu/src/copy.c,v
retrieving revision 1.140
retrieving revision 1.141
diff -u -p -u -r1.140 -r1.141
--- src/copy.c  28 Feb 2003 21:36:18 -0000      1.140
+++ src/copy.c  2 Mar 2003 06:09:28 -0000       1.141
@@ -1556,11 +1556,14 @@ copy_internal (const char *src_path, con
 
 un_backup:
 
-  /* We didn't create the destination.
-     Remove the entry associating the source dev/ino with the
+  /* We have failed to create the destination file.
+     If we've just added a dev/ino entry via the remember_copied
+     call above (i.e., unless we've just failed to create a hard link),
+     remove the entry associating the source dev/ino with the
      destination file name, so we don't try to `preserve' a link
      to a file we didn't create.  */
-  forget_created (src_sb.st_ino, src_sb.st_dev);
+  if (earlier_file == NULL)
+    forget_created (src_sb.st_ino, src_sb.st_dev);
 
   if (dst_backup)
     {
Index: tests/mv/hard-3
===================================================================
RCS file: /fetish/cu/tests/mv/hard-3,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -p -u -r1.3 -r1.4
--- tests/mv/hard-3     1 Mar 2003 21:28:39 -0000       1.3
+++ tests/mv/hard-3     2 Mar 2003 05:59:23 -0000       1.4
@@ -1,6 +1,17 @@
 #!/bin/sh
 # Ensure that using `cp --preserve=link' to copy hard-linked arguments
 # onto existing destinations works, even when one of the link operations fails.
+# This bug was fixed in coreutils-4.5.9.
+# To exercise this bug is non-trivial:
+# Set-up requires at least three hard-linked files.  In copying them,
+# while preserving links, the initial copy must succeed, the attempt
+# to create the second file via `link' must fail, and the final `link'
+# (to create the third) must succeed.  Before the corresponding fix,
+# the first and third destination file would not be linked.
+#
+# Note that this is nominally a test of `cp', yet it is in the tests/mv
+# directory, because it requires use of the --preserve=link option that
+# mv enables by default.
 
 if test "$VERBOSE" = yes; then
   set -x
Index: tests/mv/Makefile.am
===================================================================
RCS file: /fetish/cu/tests/mv/Makefile.am,v
retrieving revision 1.35
retrieving revision 1.37
diff -u -p -u -p -r1.35 -r1.37
--- tests/mv/Makefile.am        2 Feb 2003 20:15:11 -0000       1.35
+++ tests/mv/Makefile.am        2 Mar 2003 21:27:48 -0000       1.37
@@ -2,6 +2,8 @@
 AUTOMAKE_OPTIONS = 1.3 gnits
 
 TESTS = \
+  hard-3 \
+  hard-2 \
   perm-1 \
   i-link-no \
   part-fail \
Index: tests/mv/hard-3
===================================================================
RCS file: tests/mv/hard-3
diff -N tests/mv/hard-3
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/mv/hard-3     2 Mar 2003 21:29:33 -0000       1.5
@@ -0,0 +1,70 @@
+#!/bin/sh
+# Ensure that using `cp --preserve=link' to copy hard-linked arguments
+# onto existing destinations works, even when one of the link operations fails.
+# This bug was fixed in coreutils-4.5.9.
+# To exercise this bug is non-trivial:
+# Set-up requires at least three hard-linked files.  In copying them,
+# while preserving links, the initial copy must succeed, the attempt
+# to create the second file via `link' must fail, and the final `link'
+# (to create the third) must succeed.  Before the corresponding fix,
+# the first and third destination files would not be linked.
+#
+# Note that this is nominally a test of `cp', yet it is in the tests/mv
+# directory, because it requires use of the --preserve=link option that
+# mv enables by default.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  cp --version
+fi
+
+. $srcdir/../envvar-check
+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
+mkdir -p x dst/x || framework_failure=1
+touch dst/x/b || framework_failure=1
+chmod a-w dst/x
+touch a || framework_failure=1
+ln a x/b || framework_failure=1
+ln a c || framework_failure=1
+
+if test $framework_failure = 1; then
+  echo "$0: failure in testing framework" 1>&2
+  (exit 1); exit 1
+fi
+
+fail=0
+
+# ======================================
+# This must fail -- because x/b cannot be unlinked.
+cp --preserve=link --parents a x/b c dst 2> /dev/null && fail=1
+
+# Source files must remain.
+test -f a || fail=1
+test -f x/b || fail=1
+test -f c || fail=1
+cd dst
+
+# Three destination files must exist.
+test -f a || fail=1
+test -f x/b || fail=1
+test -f c || fail=1
+
+# The i-node numbers of a and c must be the same.
+ia=`ls -i a|sed 's/ a//'`
+ic=`ls -i c|sed 's/ c//'`
+test $ia = $ic || fail=1
+
+# The i-node number of x/b must be different.
+ib=`ls -i x/b|sed 's/ .*//'`
+test $ia = $ib && fail=1
+
+(exit $fail); exit $fail