Package: coreutils;
Reported by: Matt McCutchen <matt <at> mattmccutchen.net>
Date: Tue, 31 Aug 2010 21:27:01 UTC
Severity: normal
Done: Jim Meyering <jim <at> meyering.net>
Bug is archived. No further changes may be made.
Message #68 received at 6960 <at> debbugs.gnu.org (full text, mbox):
From: Jim Meyering <jim <at> meyering.net> To: Paul Eggert <eggert <at> cs.ucla.edu> Cc: 6960 <at> debbugs.gnu.org, Anders Kaseorg <andersk <at> MIT.EDU> Subject: Re: bug#6960: mv refuses to move a symlink over a hard link to the same file 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 <meyering <at> redhat.com> 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 alexey <at> pippuri.mawhrin.net Alfred M. Szmidt ams <at> kemisten.nu Ambrose Feinstein ambrose <at> google.com Amr Ali amr.ali.cc <at> gmail.com +Anders Kaseorg andersk <at> mit.edu Andi Kleen freitag <at> alancoxonachip.com Andre Novaes Cunha Andre.Cunha <at> br.global-one.net Andreas Frische andreasfrische <at> gmail.com @@ -384,6 +385,7 @@ Mate Wierdl mw <at> moni.msci.memphis.edu Matej Vela mvela <at> public.srce.hr Matias A. Fonzo selk <at> dragora.org Matt Kraai kraai <at> ftbfs.org +Matt McCutchen matt <at> mattmccutchen.net Matt Perry matt <at> primefactor.com Matt Pham mattvpham <at> gmail.com Matt Schalit mschalit <at> pacbell.net 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.