On 11/04/2013 10:37 AM, Pádraig Brady wrote:
> On 11/04/2013 12:48 AM, Bernhard Voelker wrote:
>> It got late again. I'll have a look tomorrow again - maybe there
>> are no side effects with the following:
>>
>> diff --git a/src/cp.c b/src/cp.c
>> index 7bc8630..78c0a04 100644
>> --- a/src/cp.c
>> +++ b/src/cp.c
>> @@ -1135,7 +1135,7 @@ main (int argc, char **argv)
>>
>> if (x.dereference == DEREF_UNDEFINED)
>> {
>> - if (x.recursive)
>> + if (x.recursive && ! x.hard_link)
>> /* This is compatible with FreeBSD. */
>> x.dereference = DEREF_NEVER;
>> else
>
> Yes I didn't consider -R in the table as I don't see any reason
> for it to behave differently when -l is specified (and -l aplies
> neither to POSIX or BSD (comment)). So the above adjustment
> looks correct to me.Indeed, that adjustment seems to be the right one - finally. ;-) I have added it to the patch as well as a new test case. I've added a "Co-authored-by:"-me line for that. @Gian: please note that we use "double-blank after-dot" format in comments. I've fixed that for you. The patch passes make check and syntax-check. I've also attached the test result cp-8.21 versus the new one. These are the differences (I leave out the --preserve=links cases as these are also affected but unrelated): COMMAND | 8.21 | NEW cp -l filelink ... | S | T cp -l -R filelink ... | S | T cp -L -l filelink ... | S | T cp -L -l -R filelink ... | S | T cp -H -l filelink ... | S | T cp -H -l -R filelink ... | S | T cp -l -R dirlink ... | S | NDH cp -l -R danglink ... | S | ERR_DEREF NDH = New directory with hard links below S = hard link to symlink T = hard link to symlink target ERR_DEREF = failure because dereferencing fails Now, the dereferencing behavior of cp --link is not related to -R anymore. I hope we don't introduce a regression ... Comments? Have a nice day, Berny
>From c29ed31b9bbdb8182758bc4e51604d19421bc1ec Mon Sep 17 00:00:00 2001 From: Bernhard Voelker <[email protected]> Date: Mon, 4 Nov 2013 23:37:01 +0100 Subject: [PATCH] cp: fix --link regarding the dereferencing of symbolic links * src/copy.c (create_hard_link): Add a bool 'dereference' parameter, and pass AT_SYMLINK_FOLLOW as 'flags' to linkat() when dereference is true. (should_dereference): Add new 'bool' function to determine if a file should be dereferenced or not. (copy_internal): Use the above new should_dereference() and remember its return value in a new local bool 'dereference' variable. Use that in all three calls to create_hard_link(). * src/cp.c (main): after parsing the options, if x.dereference is still DEFEF_UNDEFINED and the x.recursive is true, then only set x.dereference to DEREF_NEVER iff --link was not specified. * tests/cp/same-file.sh: Adapt the expected results for the -fl, the -bl and the -bfl tests. * tests/cp/link-deref.sh: Add a new test. * tests/local.mk (all_tests): Reference the above new test. * NEWS (Changes in behavior): Mention the change. This fixes http://bugs.gnu.org/15173 Co-authored-by: Bernhard Voelker <[email protected]> --- NEWS | 5 +++ src/copy.c | 46 +++++++++++++++------ src/cp.c | 2 +- tests/cp/link-deref.sh | 110 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/cp/same-file.sh | 6 +-- tests/local.mk | 1 + 6 files changed, 153 insertions(+), 17 deletions(-) create mode 100755 tests/cp/link-deref.sh diff --git a/NEWS b/NEWS index 39dd3d6..7333488 100644 --- a/NEWS +++ b/NEWS @@ -70,6 +70,11 @@ GNU coreutils NEWS -*- outline -*- ** Changes in behavior + cp --link now dereferences a symbolic link as source before creating the + hard link in the destination unless explicitly requested via the -P option. + Previously, it would create a hard link of the symbolic link - even when one + of the dereferencing options -L or -H was specified, too. + dd status=none now suppresses all non fatal diagnostic messages, not just the transfer counts. diff --git a/src/copy.c b/src/copy.c index f66ab46..bcae123 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1558,18 +1558,23 @@ restore_default_fscreatecon_or_die (void) _("failed to restore the default file creation context")); } -/* Create a hard link DST_NAME to SRC_NAME, honoring the REPLACE and - VERBOSE settings. Return true upon success. Otherwise, diagnose - the failure and return false. - If SRC_NAME is a symbolic link it will not be followed. If the system - doesn't support hard links to symbolic links, then DST_NAME will - be created as a symbolic link to SRC_NAME. */ +/* Create a hard link DST_NAME to SRC_NAME, honoring the REPLACE, VERBOSE and + DEREFERENCE settings. Return true upon success. Otherwise, diagnose the + failure and return false. If SRC_NAME is a symbolic link, then it will not + be followed unless DEREFERENCE is true. + If the system doesn't support hard links to symbolic links, then DST_NAME + will be created as a symbolic link to SRC_NAME. */ static bool create_hard_link (char const *src_name, char const *dst_name, - bool replace, bool verbose) + bool replace, bool verbose, bool dereference) { - /* We want to guarantee that symlinks are not followed. */ - bool link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0); + /* We want to guarantee that symlinks are not followed, unless requested. */ + int flags = 0; + if (dereference) + flags = AT_SYMLINK_FOLLOW; + + bool link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, flags) + != 0); /* If the link failed because of an existing destination, remove that file and then call link again. */ @@ -1582,7 +1587,8 @@ create_hard_link (char const *src_name, char const *dst_name, } if (verbose) printf (_("removed %s\n"), quote (dst_name)); - link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0); + link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, flags) + != 0); } if (link_failed) @@ -1595,6 +1601,17 @@ create_hard_link (char const *src_name, char const *dst_name, return true; } +/* Return true if the current file should be (tried to be) dereferenced: + either for DEREF_ALWAYS or for DEREF_COMMAND_LINE_ARGUMENTS in the case + where the current file is a COMMAND_LINE_ARG; otherwise return false. */ +static inline bool _GL_ATTRIBUTE_PURE +should_dereference (const struct cp_options *x, bool command_line_arg) +{ + return x->dereference == DEREF_ALWAYS + || (x->dereference == DEREF_COMMAND_LINE_ARGUMENTS + && command_line_arg); +} + /* Copy the file SRC_NAME to the file DST_NAME. The files may be of any type. NEW_DST should be true if the file DST_NAME cannot exist because its parent directory was just created; NEW_DST should @@ -1670,6 +1687,8 @@ copy_internal (char const *src_name, char const *dst_name, record_file (x->src_info, src_name, &src_sb); } + bool dereference = should_dereference (x, command_line_arg); + if (!new_dst) { /* Regular files can be created by writing through symbolic @@ -1748,7 +1767,7 @@ copy_internal (char const *src_name, char const *dst_name, /* Note we currently replace DST_NAME unconditionally, even if it was a newer separate file. */ if (! create_hard_link (earlier_file, dst_name, true, - x->verbose)) + x->verbose, dereference)) { goto un_backup; } @@ -2078,7 +2097,8 @@ copy_internal (char const *src_name, char const *dst_name, } else { - if (! create_hard_link (earlier_file, dst_name, true, x->verbose)) + if (! create_hard_link (earlier_file, dst_name, true, x->verbose, + dereference)) goto un_backup; return true; @@ -2389,7 +2409,7 @@ copy_internal (char const *src_name, char const *dst_name, && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode) && x->dereference == DEREF_NEVER)) { - if (! create_hard_link (src_name, dst_name, false, false)) + if (! create_hard_link (src_name, dst_name, false, false, dereference)) goto un_backup; } else if (S_ISREG (src_mode) diff --git a/src/cp.c b/src/cp.c index 7bc8630..78c0a04 100644 --- a/src/cp.c +++ b/src/cp.c @@ -1135,7 +1135,7 @@ main (int argc, char **argv) if (x.dereference == DEREF_UNDEFINED) { - if (x.recursive) + if (x.recursive && ! x.hard_link) /* This is compatible with FreeBSD. */ x.dereference = DEREF_NEVER; else diff --git a/tests/cp/link-deref.sh b/tests/cp/link-deref.sh new file mode 100755 index 0000000..105c464 --- /dev/null +++ b/tests/cp/link-deref.sh @@ -0,0 +1,110 @@ +#!/bin/sh +# Exercise cp --link's behavior regarding the dereferencing of symbolic links. + +# Copyright (C) 2013 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=.}/tests/init.sh"; path_prepend_ ./src +print_ver_ cp + +mkdir dir || framework_failure_ +cp --help > file || framework_failure_ +ln -s dir dirlink || framework_failure_ +ln -s file filelink || framework_failure_ +ln -s nowhere danglink || framework_failure_ + +# printf format of the output line. +outformat='%s|result=%s|inode=%s|type=%s|error=%s\n' + +for src in dirlink filelink danglink; do + # Get symlink's target. + tgt=$(readlink $src) || framework_failure_ + # Get inodes and file type of the symlink (src) and its target (tgt). + # Note: this will fail for 'danglink'; catch it. + ino_src="$(stat -c '%i' $src)" || framework_failure_ + typ_src="$(stat -c '%F' $src)" || framework_failure_ + ino_tgt="$(stat -c '%i' $tgt 2>/dev/null)" || ino_tgt= + typ_tgt="$(stat -c '%F' $tgt 2>/dev/null)" || typ_tgt= + + for o in '' -L -H -P; do + for r in '' -R; do + + command="cp --link $o $r $src dst" + $command 2> err + result=$? + + # Get inode and file type of the destination (which may fail, too). + ino_dst="$(stat -c '%i' dst 2>/dev/null)" || ini_dst= + typ_dst="$(stat -c '%F' dst 2>/dev/null)" || typ_dst= + + # Print the actual result in a certain format. + printf "$outformat" \ + "$command" \ + "$result" \ + "$ino_dst" \ + "$typ_dst" \ + "$(< err)" \ + > out + + # What was expected? + if [ "$o" = "-P" ]; then + # cp --link should not dereference if -P is given. + exp_result=0 + exp_inode=$ino_src + exp_ftype=$typ_src + exp_error= + elif [ "$src" = 'danglink' ]; then + # Dereferencing should fail for the 'danglink'. + exp_result=1 + exp_inode= + exp_ftype= + exp_error="cp: cannot stat 'danglink': No such file or directory" + elif [ "$src" = 'dirlink' ] && [ "$r" != '-R' ]; then + # Dereferencing should fail for the 'dirlink' without -R. + exp_result=1 + exp_inode= + exp_ftype= + exp_error="cp: omitting directory 'dirlink'" + elif [ "$src" = 'dirlink' ]; then + # cp --link -R 'dirlink' should create a new directory. + exp_result=0 + exp_inode=$ino_dst + exp_ftype=$typ_dst + exp_error= + else + # cp --link 'filelink' should create a hard link to the target. + exp_result=0 + exp_inode=$ino_tgt + exp_ftype=$typ_tgt + exp_error= + fi + + # Print the expected result in a certain format. + printf "$outformat" \ + "$command" \ + "$exp_result" \ + "$exp_inode" \ + "$exp_ftype" \ + "$exp_error" \ + > exp + + compare exp out || { ls -lid $src $tgt dst; fail=1; } + + rm -rf dst err exp out || framework_failure_ + done + done +done + +Exit $fail diff --git a/tests/cp/same-file.sh b/tests/cp/same-file.sh index 003a62b..326f9c6 100755 --- a/tests/cp/same-file.sh +++ b/tests/cp/same-file.sh @@ -189,9 +189,9 @@ cat <<\EOF | sed "$remove_these_sed" > expected 0 -bf (foo sl1 -> foo sl2 sl2.~1~ -> foo) 0 -bdf (foo sl1 -> foo sl2 -> foo sl2.~1~ -> foo) 1 -l [cp: cannot create hard link 'sl2' to 'sl1'] (foo sl1 -> foo sl2 -> foo) -0 -fl (foo sl1 -> foo sl2 -> foo) -0 -bl (foo sl1 -> foo sl2 -> foo sl2.~1~ -> foo) -0 -bfl (foo sl1 -> foo sl2 -> foo sl2.~1~ -> foo) +0 -fl (foo sl1 -> foo sl2) +0 -bl (foo sl1 -> foo sl2 sl2.~1~ -> foo) +0 -bfl (foo sl1 -> foo sl2 sl2.~1~ -> foo) 1 [cp: 'foo' and 'hardlink' are the same file] (foo hardlink) 1 -d [cp: 'foo' and 'hardlink' are the same file] (foo hardlink) diff --git a/tests/local.mk b/tests/local.mk index e18deac..3c92425 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -427,6 +427,7 @@ all_tests = \ tests/cp/file-perm-race.sh \ tests/cp/into-self.sh \ tests/cp/link.sh \ + tests/cp/link-deref.sh \ tests/cp/link-no-deref.sh \ tests/cp/link-preserve.sh \ tests/cp/link-symlink.sh \ -- 1.8.3.1
testit.log.xz
Description: application/xz
