Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au

From: Pádraig Brady <[email-masked]>
Date: Mon, 25 Jul 2011 14:35:39 +0100

On 25/07/11 12:42, Jim Meyering wrote:
> [email-masked] wrote:
>> I have tried to use the command cp combining the -a and the -u options.
>> I had to stop the copying process midways and restarted it again, and to my
>> suprice the diskusage at the destination was 10 -20 % larger than the
>> diskusage at the source and my disks ran full even though the destination
>> disks was the same size as the source disks.
> ...
> 
> Thank you for a fine bug report.
> That is indeed a bug, and it affects the latest release, coreutils-8.12.
> I confirmed that it afflicts fileutils-3.16 too, so this bug has probably
> been present since the initial implementation.
> 
> Here's the fix I expect to push:

That looks good.
Should we fold the attached patch in too?
It refactors out a create_hard_link() function,
so we wouldn't be adding any new labels.

cheers,
Pádraig.
diff --git a/src/copy.c b/src/copy.c
index c17b942..95d05f0 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1488,6 +1488,37 @@ restore_default_fscreatecon_or_die (void)
            _("failed to restore the default file creation context"));
 }
 
+static bool
+create_hard_link (char const *src_name, char const *dst_name,
+                  bool replace, bool verbose)
+{
+  /* We want to guarantee that symlinks are not followed.  */
+  bool link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0);
+
+  /* If the link failed because of an existing destination,
+     remove that file and then call link again.  */
+  if (link_failed && errno == EEXIST && replace)
+    {
+      if (unlink (dst_name) != 0)
+        {
+          error (0, errno, _("cannot remove %s"), quote (dst_name));
+          return false;
+        }
+      if (verbose)
+        printf (_("removed %s\n"), quote (dst_name));
+      link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0);
+    }
+
+  if (link_failed)
+    {
+      error (0, errno, _("cannot create hard link %s to %s"),
+             quote_n (0, dst_name), quote_n (1, src_name));
+      return false;
+    }
+
+  return true;
+}
+
 /* 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
@@ -1948,31 +1979,8 @@ copy_internal (char const *src_name, char const *dst_name,
         }
       else
         {
-          /* We want to guarantee that symlinks are not followed.  */
-          bool link_failed = (linkat (AT_FDCWD, earlier_file, AT_FDCWD,
-                                      dst_name, 0) != 0);
-
-          /* If the link failed because of an existing destination,
-             remove that file and then call link again.  */
-          if (link_failed && errno == EEXIST)
-            {
-              if (unlink (dst_name) != 0)
-                {
-                  error (0, errno, _("cannot remove %s"), quote (dst_name));
-                  goto un_backup;
-                }
-              if (x->verbose)
-                printf (_("removed %s\n"), quote (dst_name));
-              link_failed = (linkat (AT_FDCWD, earlier_file, AT_FDCWD,
-                                     dst_name, 0) != 0);
-            }
-
-          if (link_failed)
-            {
-              error (0, errno, _("cannot create hard link %s to %s"),
-                     quote_n (0, dst_name), quote_n (1, earlier_file));
-              goto un_backup;
-            }
+          if (!create_hard_link (earlier_file, dst_name, true, x->verbose))
+            goto un_backup;
 
           return true;
         }
@@ -2274,11 +2282,8 @@ copy_internal (char const *src_name, char const *dst_name,
            && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
                 && x->dereference == DEREF_NEVER))
     {
-       if (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0))
-        {
-          error (0, errno, _("cannot create link %s"), quote (dst_name));
-          goto un_backup;
-        }
+      if (!create_hard_link (src_name, dst_name, false, false))
+        goto un_backup;
     }
   else if (S_ISREG (src_mode)
            || (x->copy_as_regular && !S_ISLNK (src_mode)))