On 02/10/2014 02:29 AM, Paul Eggert wrote:
> Pádraig Brady wrote:
>> +#if ! defined HAVE_LINKAT
>>              && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
>> -                && x->dereference == DEREF_NEVER))
>> +                && x->dereference == DEREF_NEVER)
>> +#endif
> 
> Could you reword that sort of thing so as not to use the #if inside an 
> expression?  Something like this instead, perhaps, earlier in the code:
> 
>   #if !defined HAVE_LINKAT && LINK_FOLLOWS_SYMLINKS
>   # define LINKAT_FOLLOWS_SYMLINKS 1
>   #else
>   # define LINKAT_FOLLOWS_SYMLINKS 0
>   #endif

That's slightly confusing as linkat() emulation never
actually symlinks the referent.  I went for the more direct:

 #if defined HAVE_LINKAT || ! LINK_FOLLOWS_SYMLINKS
 # define CAN_HARDLINK_SYMLINKS 1
 #else
 # define CAN_HARDLINK_SYMLINKS 0
 #endif

Full patch is attached.

thanks,
Pádraig.
>From 9196a8c3375a3c8f7478b2ca8373ceeef31eaeb2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Thu, 12 Dec 2013 18:31:48 +0000
Subject: [PATCH] cp: with --link always use linkat() if available

* src/copy.c (copy_reg): If linkat() is available it doesn't
matter about the gnulib emulation provided, and thus the
LINK_FOLLOWS_SYMLINKS should not have significance here.
This was noticed on FreeBSD and the consequence is that
cp --link will create hardlinks to symlinks there, rather
than emulating with symlinks to symlinks.
* tests/cp/link-deref.sh: Adjust to the checks to cater
for all cases where hardlinks to symlinks are supported.
---
 src/copy.c             |   20 ++++++++++++++------
 tests/cp/link-deref.sh |    7 +++----
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index 3e4cbff..127eed2 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -98,6 +98,14 @@ rpl_mkfifo (char const *file, mode_t mode)
 #define SAME_GROUP(A, B) ((A).st_gid == (B).st_gid)
 #define SAME_OWNER_AND_GROUP(A, B) (SAME_OWNER (A, B) && SAME_GROUP (A, B))
 
+/* LINK_FOLLOWS_SYMLINKS is tri-state; if it is -1, we don't know
+   how link() behaves, so assume we can't hardlink symlinks in that case.  */
+#if defined HAVE_LINKAT || ! LINK_FOLLOWS_SYMLINKS
+# define CAN_HARDLINK_SYMLINKS 1
+#else
+# define CAN_HARDLINK_SYMLINKS 0
+#endif
+
 struct dir_list
 {
   struct dir_list *parent;
@@ -2484,16 +2492,15 @@ copy_internal (char const *src_name, char const *dst_name,
      should not follow the link.  We can approximate the desired
      behavior by skipping this hard-link creating block and instead
      copying the symlink, via the 'S_ISLNK'- copying code below.
-     LINK_FOLLOWS_SYMLINKS is tri-state; if it is -1, we don't know
-     how link() behaves, so we use the fallback case for safety.
 
      Note gnulib's linkat module, guarantees that the symlink is not
      dereferenced.  However its emulation currently doesn't maintain
      timestamps or ownership so we only call it when we know the
      emulation will not be needed.  */
   else if (x->hard_link
-           && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
-                && x->dereference == DEREF_NEVER))
+           && !(! CAN_HARDLINK_SYMLINKS && S_ISLNK (src_mode)
+                && x->dereference == DEREF_NEVER)
+          )
     {
       if (! create_hard_link (src_name, dst_name, false, false, dereference))
         goto un_backup;
@@ -2632,8 +2639,9 @@ copy_internal (char const *src_name, char const *dst_name,
   /* If we've just created a hard-link due to cp's --link option,
      we're done.  */
   if (x->hard_link && ! S_ISDIR (src_mode)
-      && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
-           && x->dereference == DEREF_NEVER))
+      && !(! CAN_HARDLINK_SYMLINKS && S_ISLNK (src_mode)
+           && x->dereference == DEREF_NEVER)
+     )
     return delayed_ok;
 
   if (copied_as_regular)
diff --git a/tests/cp/link-deref.sh b/tests/cp/link-deref.sh
index e56d592..5bbd8d4 100755
--- a/tests/cp/link-deref.sh
+++ b/tests/cp/link-deref.sh
@@ -20,10 +20,9 @@
 print_ver_ cp
 
 if grep '^#define HAVE_LINKAT 1' "$CONFIG_HEADER" > /dev/null \
-   && grep '^#define LINK_FOLLOWS_SYMLINKS 0' "$CONFIG_HEADER" > /dev/null; then
-  # With this config (which is the case on GNU/Linux) cp will attempt to
-  # linkat() to hardlink a symlink.  So now see if the current file system
-  # supports this operation.
+   || grep '^#define LINK_FOLLOWS_SYMLINKS 0' "$CONFIG_HEADER" > /dev/null; then
+  # With this config cp will attempt to linkat() to hardlink a symlink.
+  # So now see if the current file system supports this operation.
   ln -s testtarget test_sl || framework_failure_
   ln -P test_sl test_hl_sl || framework_failure_
   ino_sl="$(stat -c '%i' test_sl)" || framework_failure_
-- 
1.7.7.6

Reply via email to