On 10/30/2013 01:13 PM, Bernhard Voelker wrote: > Thanks again for looking into this. It's really quite complex. > I'll come up with the test (old vs. new) cases soon.
Okay, here we go:
the attached patch is updated regarding the issues you mentioned.
I furthermore tweaked the NEWS entry:
* instead of just mentioning the --dereference option,
mention both the -L and -H options explicitly.
* add a note that "cp -l dangling" and "cp -l symlink-to-dir"
now create hard links as expected - cp doesn't try to dereference
the src anymore which would lead to different error diagnostics
in the two cases [*].
Then, I created a few test cases to check the changes in behaviour
of cp-8.21 against the changed cp. It runs various combinations of
the options -l, -s, -R, -H, -L, -P and --preserve=links on symlinks
to a file ('filelink'), to a directory ('dirlink') and on a dangling
symlink ('danglink').
The script is already ugly enough, so I omitted checking the
files, dir and links in the subdirectory for changes.
The changes so far - 12 out of 144 - are:
$ grep "DIFFERENCE in Test: " testit.log | sed 's/^.*: //'
cp -L -l filelink ...
cp -L -l --preserve=links filelink ...
cp -L -l -R filelink ...
cp -L -l -R --preserve=links filelink ...
cp -H -l filelink ...
cp -H -l --preserve=links filelink ...
cp -H -l -R filelink ...
cp -H -l -R --preserve=links filelink ...
cp -l dirlink ...
cp -l --preserve=links dirlink ...
cp -l danglink ...
cp -l --preserve=links danglink ...
I think these are all okay.
[*] IMO even the 4 latter changes are okay and don't contradict to
POSIX: --link is not specified by POSIX.
BTW: The failures "can make relative symbolic links only in current directory"
for e.g. "cp -L -s -R dirlink" are interesting but a different case, i.e.,
cp's behavior didn't change.
WDYT?
Finally, the testsuite is still broken (tests/cp/same-file), and new
tests for "cp -l[LH]" are also still missing.
Have a nice day,
Berny
>From 49d2687d0f961f7df3d8e32995ff054c72fbf1d9 Mon Sep 17 00:00:00 2001 From: Bernhard Voelker <[email protected]> Date: Thu, 31 Oct 2013 02:24:03 +0100 Subject: [PATCH] cp: fix --link with dereferencing options -L or -H [DRAFT] * 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 pass its return value as the new parameter 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 --link option was specified, initialize x.dereference to DEREF_NEVER. * NEWS (Changes in behavior): Mention the change. This fixes http://bugs.gnu.org/15173 --- NEWS | 7 +++++++ src/copy.c | 47 ++++++++++++++++++++++++++++++++++------------- src/cp.c | 2 +- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index 39dd3d6..cbd837d 100644 --- a/NEWS +++ b/NEWS @@ -70,6 +70,13 @@ GNU coreutils NEWS -*- outline -*- ** Changes in behavior + cp --link together with -H or -L now dereferences a symbolic link as source + before creating the hard link in the destination. Previously, it would create + a hard link of the symbolic link. + Similarly, cp --link for a dangling symbolic link or for a symbolic link to a + directory is now hard linked correctly. Previously, cp would fail with an + error diagnostic. + 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..e19fb34 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1558,18 +1558,24 @@ 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 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); + int flags; + if (dereference) + flags = AT_SYMLINK_FOLLOW; + else + flags = 0; /* We want to guarantee that symlinks are not followed. */ + + 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 +1588,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 +1602,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 CLI_ARG; otherwise return false. */ +static inline bool _GL_ATTRIBUTE_PURE +should_dereference (const struct cp_options *x, bool cli_arg) +{ + return x->dereference == DEREF_ALWAYS + || (x->dereference == DEREF_COMMAND_LINE_ARGUMENTS + && cli_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 @@ -1747,8 +1765,9 @@ 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. */ + bool deref = should_dereference (x, command_line_arg); if (! create_hard_link (earlier_file, dst_name, true, - x->verbose)) + x->verbose, deref)) { 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, + should_dereference (x, command_line_arg))) goto un_backup; return true; @@ -2389,7 +2409,8 @@ 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, + should_dereference (x, command_line_arg))) goto un_backup; } else if (S_ISREG (src_mode) diff --git a/src/cp.c b/src/cp.c index 7bc8630..6efb152 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 -- 1.8.3.1
testit.log.xz
Description: application/xz
testit.sh
Description: application/shellscript
