From 63feb84a2db5246fb71df45884589b914679110c Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Wed, 19 Mar 2008 13:37:04 +0100 Subject: mv: never unlink a destination file before calling rename While cp --preserve=links must unlink certain destination files, mv must never do that. * src/copy.c (copy_internal): Pull the '! x->move_mode' test "up", so it affects the entire condition, and not just DEREF_NEVER mode. Reported by James Ralston in . * tests/mv/atomic2: New file. Test for the above fix. * tests/mv/Makefile.am (TESTS): Add atomic2. * NEWS: Mention the bug-fix. [Bug introduced in 367719ba5f1dbd5e2f7fa2466c441f23f66a7c9e] --- NEWS | 4 ++++ THANKS | 1 + src/copy.c | 5 +++-- tests/mv/Makefile.am | 4 ++-- tests/mv/atomic2 | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 60 insertions(+), 4 deletions(-) create mode 100755 tests/mv/atomic2 diff --git a/NEWS b/NEWS index 948bcede7..93f133123 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,10 @@ GNU coreutils NEWS -*- outline -*- ls no longer segfaults on files in /proc when linked with an older version of libselinux. E.g., ls -l /proc/sys would dereference a NULL pointer. + mv would mistakenly unlink a destination file before calling rename, + when the destination had two or more hard links. It no longer does that. + [bug introduced in coreutils-5.3.0] + "rmdir --ignore-fail-on-non-empty" detects and ignores the failure in more cases when a directory is empty. diff --git a/THANKS b/THANKS index 2a47910c8..186bf5f58 100644 --- a/THANKS +++ b/THANKS @@ -220,6 +220,7 @@ James james@albion.glarp.com James Antill jmanti%essex.ac.uk@seralph21.essex.ac.uk James Lemley James.Lemley@acxiom.com James Hunt jamesodhunt@hotmail.com +James Ralston ralston@pobox.com James Sneeringer jvs@ocslink.com James Tanis jtt@soscorp.com James Youngman james+usenet@free-lunch.demon.co.uk diff --git a/src/copy.c b/src/copy.c index fd31b5c91..208a67413 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1339,10 +1339,11 @@ copy_internal (char const *src_name, char const *dst_name, new_dst = true; } else if (! S_ISDIR (dst_sb.st_mode) + /* Never unlink dst_name when in move mode. */ + && ! x->move_mode && (x->unlink_dest_before_opening || (x->preserve_links && 1 < dst_sb.st_nlink) - || (!x->move_mode - && x->dereference == DEREF_NEVER + || (x->dereference == DEREF_NEVER && S_ISLNK (src_sb.st_mode)) )) { diff --git a/tests/mv/Makefile.am b/tests/mv/Makefile.am index c1219113e..92ec68e39 100644 --- a/tests/mv/Makefile.am +++ b/tests/mv/Makefile.am @@ -1,7 +1,6 @@ # Make coreutils tests for "mv". -*-Makefile-*- -# Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007 -# Free Software Foundation, Inc. +# Copyright (C) 1998-2008 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 @@ -17,6 +16,7 @@ # along with this program. If not, see . TESTS = \ + atomic2 \ sticky-to-xpart \ hard-verbose \ backup-dir \ diff --git a/tests/mv/atomic2 b/tests/mv/atomic2 new file mode 100755 index 000000000..d1029aa9c --- /dev/null +++ b/tests/mv/atomic2 @@ -0,0 +1,50 @@ +#!/bin/sh +# ensure that mv doesn't first unlink a multi-hard-linked destination + +# Copyright (C) 2008 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 . + +if test "$VERBOSE" = yes; then + set -x + mv --version +fi + +. $srcdir/../test-lib.sh +require_strace_ + +# Before the fix, mv would unnecessarily unlink the destination symlink: +# $ rm -f a b b2; touch a b; ln b b2; strace -e unlink /p/bin/mv a b +# unlink("b") = 0 +# +# With the fix, it doesn't call unlink: +# $ rm -f a b b2; touch a b; ln b b2; strace -e unlink ./mv a b +# $ + +touch a b || framework_failure +ln b b2 || framework_failure + +fail=0 + +strace -qe unlink mv a b > out 2>&1 || fail=1 +$EGREP 'unlink.*"b"' out && fail=1 + +# Ensure that the source, "a", is gone. +ls -dl a > /dev/null 2>&1 && fail=1 + +# Ensure that the destination, "b", has link count 1. +n_links=`stat --printf=%h b` || fail=1 +test "$n_links" = 1 || fail=1 + +(exit $fail); exit $fail -- cgit v1.2.3-70-g09d2