mv: allow moving symlink onto same-inode dest with >= 2 hard links
authorJim Meyering <meyering@redhat.com>
Thu, 5 Jan 2012 10:45:50 +0000 (11:45 +0100)
committerJim Meyering <meyering@redhat.com>
Mon, 30 Jan 2012 19:43:07 +0000 (20:43 +0100)
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.
Improved-by: Paul Eggert.
NEWS
THANKS.in
src/copy.c
tests/Makefile.am
tests/mv/symlink-onto-hardlink [new file with mode: 0755]

diff --git a/NEWS b/NEWS
index 2b0926f..9eebbf6 100644 (file)
--- 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]
 
index 13c8817..904bb3e 100644 (file)
--- a/THANKS.in
+++ b/THANKS.in
@@ -39,6 +39,7 @@ Alexey Vyskubov                     alexey@pippuri.mawhrin.net
 Alfred M. Szmidt                    ams@kemisten.nu
 Ambrose Feinstein                   ambrose@google.com
 Amr Ali                             amr.ali.cc@gmail.com
+Anders Kaseorg                      andersk@mit.edu
 Andi Kleen                          freitag@alancoxonachip.com
 Andre Novaes Cunha                  Andre.Cunha@br.global-one.net
 Andreas Frische                     andreasfrische@gmail.com
@@ -384,6 +385,7 @@ Mate Wierdl                         mw@moni.msci.memphis.edu
 Matej Vela                          mvela@public.srce.hr
 Matias A. Fonzo                     selk@dragora.org
 Matt Kraai                          kraai@ftbfs.org
+Matt McCutchen                      matt@mattmccutchen.net
 Matt Perry                          matt@primefactor.com
 Matt Pham                           mattvpham@gmail.com
 Matt Schalit                        mschalit@pacbell.net
index 51f51be..4810de8 100644 (file)
@@ -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,39 @@ 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
+     1) src is a symlink,
+     2) dest has a link count of 2 or more and
+     3) dest and the referent of src are not the same directory entry,
+     then it's ok, since while we'll lose one of those hard links,
+     src will still point to a remaining link.
+     Note that technically, condition #3 obviates condition #2, but we
+     retain the 1 < st_nlink condition because that means fewer invocations
+     of the more expensive #3.
+
+     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)
+      && 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 +1871,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)
index 8b670fc..a94aaa2 100644 (file)
@@ -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 (executable)
index 0000000..2dac484
--- /dev/null
@@ -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