Subject: bug#6960: mv refuses to move a symlink over a hard link to the same file

From: Jim Meyering <[email-masked]>
Date: Sun, 29 Jan 2012 22:33:05 +0100

Paul Eggert wrote:
> On 01/04/12 14:30, Jim Meyering wrote:
>> You could form the symlink-free full name of the referent, abs_src
>> and then test same_name (abs_src, dst_name).
>
> That doesn't sound right, since the symlink may resolve differently
> after it's moved.  For example:

Hi Paul,

Interesting point.  Thanks.

> $ ls -ldt lt ny d d/lt.new
> drwxr-xr-x. 2 eggert eggert 4096 Jan  4 14:44 d
> lrwxrwxrwx. 1 eggert eggert    2 Jan  4 14:26 d/lt.new -> ny
> -rw-r--r--. 2 eggert eggert    2 Jan  4 14:26 lt
> -rw-r--r--. 2 eggert eggert    2 Jan  4 14:26 ny
> $ mv d/lt.new ny

However, your example is not relevant, because the source is
a mere dangling symlink.  Thus, it doesn't meet the requirements
for that similarity check to be invoked.

Perhaps you meant something like this:

    $ rm -f [dfgs]; mkdir d; touch f; ln f g; ln -s ../f d/k; env mv d/k g
    mv: 'd/k' and 'g' are the same file

    $ ls -ogi f g d/k
    75574240 lrwxrwxrwx. 1 4 Jan 29 19:43 d/k -> ../f
    75574239 -rw-------. 2 0 Jan 29 19:43 f
    75574239 -rw-------. 2 0 Jan 29 19:43 g

And in that case, I find it hard to imagine a legitimate
use in which one would want to move such a relative symlink
onto its referent at a different level of the hierarchy.
So maybe we don't need to add a conjunct for that case after all.

...
> I'm becoming more inclined to think that Anders's argument:
>
>> mv is already perfectly happy to atomically overwrite a
>> regular file with a symlink (even if that causes data loss)
>
> is a valid one.

If your/Anders' point is that moving an arbitrary symlink onto
an unrelated regular file can unlink the final copy of the target,
well, yes, that's true, but there's nothing subtle or unusual about
that case, while there is in the cases that mv does reject.

mv calls rename, so of course it must induce data loss.
The point of this exception is to reject the very few
cases that indicate user error with very high probability.

Here's a more complete patch:

>From 799b7f7c6f27a5356e76861066dfa554d3951f0d Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Thu, 5 Jan 2012 11:45:50 +0100
Subject: [PATCH] mv: allow moving symlink onto same-inode dest with >= 2 hard
 links

Normally, mv detects a few subtle cases in which proceeding with a
same-file rename would, with very high probability, cause data loss.
Here, we have found a corner case in which one of these same-inode
tests makes mv refuse to perform a useful operation.  Permit that
corner case.
* src/copy.c (same_file_ok): Detect/exempt this case.
* tests/mv/symlink-onto-hardlink: New test.
* tests/Makefile.am (TESTS): Add it.
* NEWS (Bug fixes): Mention it.
Initially reported by: Matt McCutchen in http://bugs.gnu.org/6960.
Raised again by Anders Kaseorg due to http://bugs.debian.org/654596.
---
 NEWS                           |    9 ++++++++
 THANKS.in                      |    2 +
 src/copy.c                     |   37 ++++++++++++++++++++++++++++++++++++
 tests/Makefile.am              |    1 +
 tests/mv/symlink-onto-hardlink |   41 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 90 insertions(+), 0 deletions(-)
 create mode 100755 tests/mv/symlink-onto-hardlink

diff --git a/NEWS b/NEWS
index 2b0926f..9eebbf6 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,15 @@ GNU coreutils NEWS                                    -*- outline -*-

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

+** Bug fixes
+
+  mv now lets you move a symlink onto a same-inode destination file that
+  has two or more hard links.  Before, it would reject that, saying that
+  they are the same, implicitly warning you that the move would result in
+  data loss.  In this unusual case, when not moving the symlink onto its
+  referent, there is no risk of data loss, since the symlink will
+  typically still point to one of the hard links.
+

 * Noteworthy changes in release 8.15 (2012-01-06) [stable]

diff --git a/THANKS.in b/THANKS.in
index 13c8817..904bb3e 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -39,6 +39,7 @@ Alexey Vyskubov                     [email-masked]
 Alfred M. Szmidt                    [email-masked]
 Ambrose Feinstein                   [email-masked]
 Amr Ali                             [email-masked]
+Anders Kaseorg                      [email-masked]
 Andi Kleen                          [email-masked]
 Andre Novaes Cunha                  [email-masked]
 Andreas Frische                     [email-masked]
@@ -384,6 +385,7 @@ Mate Wierdl                         [email-masked]
 Matej Vela                          [email-masked]
 Matias A. Fonzo                     [email-masked]
 Matt Kraai                          [email-masked]
+Matt McCutchen                      [email-masked]
 Matt Perry                          [email-masked]
 Matt Pham                           [email-masked]
 Matt Schalit                        [email-masked]
diff --git a/src/copy.c b/src/copy.c
index 51f51be..af79ed3 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -34,6 +34,7 @@
 #include "acl.h"
 #include "backupfile.h"
 #include "buffer-lcm.h"
+#include "canonicalize.h"
 #include "copy.h"
 #include "cp-hash.h"
 #include "extent-scan.h"
@@ -1349,6 +1350,38 @@ same_file_ok (char const *src_name, struct stat const *src_sb,
         }
     }

+  /* At this point, it is normally an error (data loss) to move a symlink
+     onto its referent, but in at least one narrow case, it is not:
+     In move mode, when
+     src is a symlink,
+     dest is not a symlink,
+     dest has a link count of 2 or more and
+     dest and the referent of src are not the same entry,
+     then it's ok, since while we'll lose one of those hard links,
+     src will still point to a remaining link.
+
+     Given this,
+       $ touch f && ln f l && ln -s f s
+       $ ls -og f l s
+       -rw-------. 2  0 Jan  4 22:46 f
+       -rw-------. 2  0 Jan  4 22:46 l
+       lrwxrwxrwx. 1  1 Jan  4 22:46 s -> f
+     this must fail: mv s f
+     this must succeed: mv s l */
+  if (x->move_mode
+      && S_ISLNK (src_sb->st_mode)
+      && ! S_ISLNK (dst_sb->st_mode)
+      && 1 < dst_sb_link->st_nlink)
+    {
+      char *abs_src = canonicalize_file_name (src_name);
+      if (abs_src)
+        {
+          bool result = ! same_name (abs_src, dst_name);
+          free (abs_src);
+          return result;
+        }
+    }
+
   /* It's ok to remove a destination symlink.  But that works only when we
      unlink before opening the destination and when the source and destination
      files are on the same partition.  */
@@ -1837,6 +1870,10 @@ copy_internal (char const *src_name, char const *dst_name,
                  to use fts, so using alloca here will be less of a problem.  */
               ASSIGN_STRDUPA (dst_backup, tmp_backup);
               free (tmp_backup);
+              /* In move mode, when src_name and dst_name are on the
+                 same partition (FIXME, and when they are non-directories),
+                 make the operation atomic: link dest
+                 to backup, then rename src to dest.  */
               if (rename (dst_name, dst_backup) != 0)
                 {
                   if (errno != ENOENT)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8b670fc..a94aaa2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -491,6 +491,7 @@ TESTS =						\
   mv/part-symlink				\
   mv/partition-perm				\
   mv/perm-1					\
+  mv/symlink-onto-hardlink			\
   mv/to-symlink					\
   mv/trailing-slash				\
   mv/update					\
diff --git a/tests/mv/symlink-onto-hardlink b/tests/mv/symlink-onto-hardlink
new file mode 100755
index 0000000..2dac484
--- /dev/null
+++ b/tests/mv/symlink-onto-hardlink
@@ -0,0 +1,41 @@
+#!/bin/sh
+# Ensure that mv works with a few symlink-onto-hard-link cases.
+
+# Copyright (C) 2012 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 3 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, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+print_ver_ mv
+
+touch f || framework_failure_
+ln f h || framework_failure_
+ln -s f s || framework_failure_
+
+# Given two links f and h to some important content, and a symlink s to f,
+# "mv s f" must fail because it might then be hard to find the link, h.
+# "mv s l" may succeed because then, s (now "l") still points to f.
+# Of course, if the symlink were being moved into a different destination
+# directory, things would be very different, and, I suspect, implausible.
+
+echo "mv: 's' and 'f' are the same file" > exp || framework_failure_
+mv s f > out 2> err && fail=1
+compare /dev/null out || fail=1
+compare exp err || fail=1
+
+mv s l > out 2> err || fail=1
+compare /dev/null out || fail=1
+compare /dev/null err || fail=1
+
+Exit $fail
--
1.7.9.1.g63eb