From d506af44befdbeabb027322652d4842314ba07be Mon Sep 17 00:00:00 2001 From: =?utf8?q?P=C3=A1draig=20Brady?= Date: Tue, 12 Jan 2016 12:39:43 +0000 Subject: [PATCH] mv: fix data loss with repeated source dir and same destination commit v8.23-31-g90aa291 failed to consider this case, where the previous rename has failed, thus causing the following to remove the specified directory: mv dir dir dir * src/copy.c (copy_internal): Assume this rename attempt has succeeded, as a previous failure will already have been handled, and we don't want to remove the source directory in this case. * tests/cp/duplicate-sources.sh: Consolidate this test file to... * tests/mv/dup-source.sh: ...here. Add test cases for same source and dest. * tests/local.mk: Remove the consolidated test. * NEWS: Mention the bug fix. Reported at https://bugzilla.redhat.com/1297464 --- NEWS | 4 +++ src/copy.c | 12 ++++++--- tests/cp/duplicate-sources.sh | 44 --------------------------------- tests/local.mk | 1 - tests/mv/dup-source.sh | 46 +++++++++++++++++++++++++++-------- 5 files changed, 48 insertions(+), 59 deletions(-) delete mode 100755 tests/cp/duplicate-sources.sh diff --git a/NEWS b/NEWS index a57c4b36fd..5dbf941d63 100644 --- a/NEWS +++ b/NEWS @@ -19,6 +19,10 @@ GNU coreutils NEWS -*- outline -*- ls no longer prematurely wraps lines when printing short file names. [bug introduced in coreutils-5.1.0] + mv no longer causes data loss due to removing a source directory specified + multiple times, when that directory is also specified as the destination. + [bug introduced in coreutils-8.24] + shred again uses defined patterns for all iteration counts. [bug introduced in coreutils-5.93] diff --git a/src/copy.c b/src/copy.c index aeb366a793..db2ce73774 100644 --- a/src/copy.c +++ b/src/copy.c @@ -2278,10 +2278,14 @@ copy_internal (char const *src_name, char const *dst_name, error (0, 0, _("warning: source directory %s " "specified more than once"), quoteaf (top_level_src_name)); - /* We only do backups in move mode and for non dirs, - and in move mode this won't be the issue as the source will - be missing for subsequent attempts. - There we just warn and return here. */ + /* In move mode, if a previous rename succeeded, then + we won't be in this path as the source is missing. If the + rename previously failed, then that has been handled, so + pretend this attempt succeeded so the source isn't removed. */ + if (x->move_mode && rename_succeeded) + *rename_succeeded = true; + /* We only do backups in move mode, and for non directories. + So just ignore this repeated entry. */ return true; } else if (x->dereference == DEREF_ALWAYS diff --git a/tests/cp/duplicate-sources.sh b/tests/cp/duplicate-sources.sh deleted file mode 100755 index c0170bb116..0000000000 --- a/tests/cp/duplicate-sources.sh +++ /dev/null @@ -1,44 +0,0 @@ -#!/bin/sh -# Ensure cp warns about but otherwise ignores source -# items specified multiple times. - -# Copyright (C) 2014-2016 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 . - -. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src -print_ver_ cp - -mkdir a || framework_failure_ -touch f || framework_failure_ - -# verify multiple files and dir sources only warned about -mkdir dest || framework_failure_ -cp -a a a f f dest 2>err || fail=1 -rm -Rf dest || framework_failure_ - -# verify multiple dirs and files with different names copied -mkdir dest || framework_failure_ -ln -s a al || framework_failure_ -ln -s f fl || framework_failure_ -cp -aH a al f fl dest 2>>err || fail=1 - -cat <exp -cp: warning: source directory 'a' specified more than once -cp: warning: source file 'f' specified more than once -EOF - -compare exp err || fail=1 - -Exit $fail diff --git a/tests/local.mk b/tests/local.mk index aa85e26f11..8898897d0e 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -443,7 +443,6 @@ all_tests = \ tests/cp/dir-rm-dest.sh \ tests/cp/dir-slash.sh \ tests/cp/dir-vs-file.sh \ - tests/cp/duplicate-sources.sh \ tests/cp/existing-perm-dir.sh \ tests/cp/existing-perm-race.sh \ tests/cp/fail-perm.sh \ diff --git a/tests/mv/dup-source.sh b/tests/mv/dup-source.sh index ff9de9dfcc..efa4624393 100755 --- a/tests/mv/dup-source.sh +++ b/tests/mv/dup-source.sh @@ -24,25 +24,37 @@ print_ver_ cp mv skip_if_root_ +reset_files() { rm -fr a b d; touch a; mkdir b d; } + for i in cp; do # cp may not fail in this case. - - rm -fr a d; touch a; mkdir d + reset_files $i a a d/ 2> out || fail=1 - rm -fr a d; touch a; mkdir d + reset_files $i ./a a d/ 2>> out || fail=1 + # Similarly for directories, but handle + # source == dest appropriately. + reset_files + $i -a ./b b d/ 2>> out || fail=1 + reset_files + returns_ 1 $i -a ./b b b/ 2>> out || fail=1 + # cp succeeds with --backup=numbered. - rm -fr a d; touch a; mkdir d + reset_files $i --backup=numbered a a d/ 2>> out || fail=1 # But not with plain '--backup' - rm -fr a d; touch a; mkdir d - $i --backup a a d/ 2>> out && fail=1 + reset_files + returns_ 1 $i --backup a a d/ 2>> out || fail=1 + cat < exp $i: warning: source file 'a' specified more than once $i: warning: source file 'a' specified more than once +$i: warning: source directory 'b' specified more than once +$i: cannot copy a directory, './b', into itself, 'b/b' +$i: warning: source directory 'b' specified more than once $i: will not overwrite just-created 'd/a' with 'a' EOF compare exp out || fail=1 @@ -50,14 +62,28 @@ done for i in mv; do # But mv *does* fail in this case (it has to). + reset_files + returns_ 1 $i a a d/ 2> out || fail=1 + returns_ 1 test -e a || fail=1 + reset_files + returns_ 1 $i ./a a d/ 2>> out || fail=1 + returns_ 1 test -e a || fail=1 + + # Similarly for directories, also handling + # source == dest appropriately. + reset_files + returns_ 1 $i ./b b d/ 2>> out || fail=1 + returns_ 1 test -e b || fail=1 + reset_files + returns_ 1 $i --verbose ./b b b/ 2>> out || fail=1 + test -d b || fail=1 - rm -fr a d; touch a; mkdir d - $i a a d/ 2> out && fail=1 - rm -fr a d; touch a; mkdir d - $i ./a a d/ 2>> out && fail=1 cat < exp $i: cannot stat 'a': No such file or directory $i: cannot stat 'a': No such file or directory +$i: cannot stat 'b': No such file or directory +$i: cannot move './b' to a subdirectory of itself, 'b/b' +$i: warning: source directory 'b' specified more than once EOF compare exp out || fail=1 done -- 2.47.2