GNU bug report logs - #33096
[INSTALLED] ln: avoid directory hard-link races

Previous Next

Package: coreutils;

Reported by: Paul Eggert <eggert <at> cs.ucla.edu>

Date: Fri, 19 Oct 2018 19:44:01 UTC

Severity: normal

Done: Paul Eggert <eggert <at> cs.ucla.edu>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 33096 in the body.
You can then email your comments to 33096 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-coreutils <at> gnu.org:
bug#33096; Package coreutils. (Fri, 19 Oct 2018 19:44:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Eggert <eggert <at> cs.ucla.edu>:
New bug report received and forwarded. Copy sent to bug-coreutils <at> gnu.org. (Fri, 19 Oct 2018 19:44:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-coreutils <at> gnu.org
Cc: Paul Eggert <eggert <at> cs.ucla.edu>
Subject: [INSTALLED] ln: avoid directory hard-link races
Date: Fri, 19 Oct 2018 12:43:08 -0700
Previously, 'ln A B' did 'stat("B"), lstat("A"), link("A","B")'
where the stat and lstat were necessary to avoid hard-linking
directories on systems that can hard-link directories.
Now, in situations that prohibit hard links to directories,
'ln A B' merely does 'link("A","B")'.  The new behavior
avoids some races and should be more efficient.
This patch was inspired by Bug#10020, which was about 'ln'.
* bootstrap.conf (gnulib_modules): Add unlinkdir.
* src/force-link.c (force_linkat, force_symlinkat): New arg for
error number of previous try.  Return error number, 0, or -1 if
error, success, or success after removal.  All callers changed.
* src/ln.c: Include priv-set.h, unlinkdir.h.
(beware_hard_dir_link): New static var.
(errnoize, atomic_link): New functions.
(target_directory_operand): Use errnoize for simplicity.
(do_link): New arg for error number of previous try.  All callers
changed.  Do each link atomically if possible.
(main): Do -r check earlier.  Remove linkdir privileges so we can
use a single linkat/symlinkat instead of a racy substitute for the
common case of 'ln A B' and 'ln -s A B'.  Set beware_hard_dir_link
to disable this optimization.
---
 NEWS             |   6 +
 bootstrap.conf   |   1 +
 src/copy.c       |  30 ++---
 src/force-link.c |  54 ++++----
 src/force-link.h |   4 +-
 src/ln.c         | 333 ++++++++++++++++++++++++++---------------------
 6 files changed, 239 insertions(+), 189 deletions(-)

diff --git a/NEWS b/NEWS
index 0fea1a118..ae9667e61 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,12 @@ GNU coreutils NEWS                                    -*- outline -*-
 
   df no longer corrupts displayed multibyte characters on macOS.
 
+  When possible 'ln A B' now merely links A to B and reports an error
+  if this fails, instead of statting A and B before linking.  This
+  uses fewer system calls and avoids some races.  The old statting
+  approach is still used in situations where hard links to directories
+  are allowed (e.g., NetBSD when superuser).
+
 ** New features
 
   id now supports specifying multiple users.
diff --git a/bootstrap.conf b/bootstrap.conf
index fcf29dc23..f193a5825 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -257,6 +257,7 @@ gnulib_modules="
   unistd-safer
   unlink-busy
   unlinkat
+  unlinkdir
   unlocked-io
   unsetenv
   update-copyright
diff --git a/src/copy.c b/src/copy.c
index 417147bc2..1f4d47737 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1783,16 +1783,16 @@ static bool
 create_hard_link (char const *src_name, char const *dst_name,
                   bool replace, bool verbose, bool dereference)
 {
-  int status = force_linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name,
-                             dereference ? AT_SYMLINK_FOLLOW : 0,
-                             replace);
-  if (status < 0)
+  int err = force_linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name,
+                          dereference ? AT_SYMLINK_FOLLOW : 0,
+                          replace, -1);
+  if (0 < err)
     {
-      error (0, errno, _("cannot create hard link %s to %s"),
+      error (0, err, _("cannot create hard link %s to %s"),
              quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
       return false;
     }
-  if (0 < status && verbose)
+  if (err < 0 && verbose)
     printf (_("removed %s\n"), quoteaf (dst_name));
   return true;
 }
@@ -2630,11 +2630,12 @@ copy_internal (char const *src_name, char const *dst_name,
               goto un_backup;
             }
         }
-      if (force_symlinkat (src_name, AT_FDCWD, dst_name,
-                           x->unlink_dest_after_failed_open)
-          < 0)
+
+      int err = force_symlinkat (src_name, AT_FDCWD, dst_name,
+                                 x->unlink_dest_after_failed_open, -1);
+      if (0 < err)
         {
-          error (0, errno, _("cannot create symbolic link %s to %s"),
+          error (0, err, _("cannot create symbolic link %s to %s"),
                  quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
           goto un_backup;
         }
@@ -2713,10 +2714,9 @@ copy_internal (char const *src_name, char const *dst_name,
           goto un_backup;
         }
 
-      int symlink_r = force_symlinkat (src_link_val, AT_FDCWD, dst_name,
-                                       x->unlink_dest_after_failed_open);
-      int symlink_err = symlink_r < 0 ? errno : 0;
-      if (symlink_err && x->update && !new_dst && S_ISLNK (dst_sb.st_mode)
+      int symlink_err = force_symlinkat (src_link_val, AT_FDCWD, dst_name,
+                                         x->unlink_dest_after_failed_open, -1);
+      if (0 < symlink_err && x->update && !new_dst && S_ISLNK (dst_sb.st_mode)
           && dst_sb.st_size == strlen (src_link_val))
         {
           /* See if the destination is already the desired symlink.
@@ -2733,7 +2733,7 @@ copy_internal (char const *src_name, char const *dst_name,
             }
         }
       free (src_link_val);
-      if (symlink_err)
+      if (0 < symlink_err)
         {
           error (0, symlink_err, _("cannot create symbolic link %s"),
                  quoteaf (dst_name));
diff --git a/src/force-link.c b/src/force-link.c
index 1794a5786..f2242fc36 100644
--- a/src/force-link.c
+++ b/src/force-link.c
@@ -85,22 +85,27 @@ try_link (char *dest, void *arg)
 
 /* Hard-link directory SRCDIR's file SRCNAME to directory DSTDIR's
    file DSTNAME, using linkat-style FLAGS to control the linking.
-   If FORCE and DSTNAME already exists, replace it atomically.  Return
-   1 if successful and DSTNAME already existed,
+   If FORCE and DSTNAME already exists, replace it atomically.
+   If LINKAT_ERRNO is 0, the hard link is already done; if positive,
+   the hard link was tried and failed with errno == LINKAT_ERRNO.  Return
+   -1 if successful and DSTNAME already existed,
    0 if successful and DSTNAME did not already exist, and
-   -1 (setting errno) on failure.  */
+   a positive errno value on failure.  */
 extern int
 force_linkat (int srcdir, char const *srcname,
-              int dstdir, char const *dstname, int flags, bool force)
+              int dstdir, char const *dstname, int flags, bool force,
+              int linkat_errno)
 {
-  int r = linkat (srcdir, srcname, dstdir, dstname, flags);
-  if (!force || r == 0 || errno != EEXIST)
-    return r;
+  if (linkat_errno < 0)
+    linkat_errno = (linkat (srcdir, srcname, dstdir, dstname, flags) == 0
+                    ? 0 : errno);
+  if (!force || linkat_errno != EEXIST)
+    return linkat_errno;
 
   char buf[smallsize];
   char *dsttmp = samedir_template (dstname, buf);
   if (! dsttmp)
-    return -1;
+    return errno;
   struct link_arg arg = { srcdir, srcname, dstdir, flags };
   int err;
 
@@ -108,7 +113,7 @@ force_linkat (int srcdir, char const *srcname,
     err = errno;
   else
     {
-      err = renameat (dstdir, dsttmp, dstdir, dstname) == 0 ? 0 : errno;
+      err = renameat (dstdir, dsttmp, dstdir, dstname) == 0 ? -1 : errno;
       /* Unlink DSTTMP even if renameat succeeded, in case DSTTMP
          and DSTNAME were already the same hard link and renameat
          was a no-op.  */
@@ -117,10 +122,7 @@ force_linkat (int srcdir, char const *srcname,
 
   if (dsttmp != buf)
     free (dsttmp);
-  if (!err)
-    return 1;
-  errno = err;
-  return -1;
+  return err;
 }
 
 
@@ -140,22 +142,25 @@ try_symlink (char *dest, void *arg)
 }
 
 /* Create a symlink containing SRCNAME in directory DSTDIR's file DSTNAME.
-   If FORCE and DSTNAME already exists, replace it atomically.  Return
-   1 if successful and DSTNAME already existed,
+   If FORCE and DSTNAME already exists, replace it atomically.
+   If SYMLINKAT_ERRNO is 0, the symlink is already done; if positive,
+   the symlink was tried and failed with errno == SYMLINKAT_ERRNO.  Return
+   -1 if successful and DSTNAME already existed,
    0 if successful and DSTNAME did not already exist, and
-   -1 (setting errno) on failure.  */
+   a positive errno value on failure.  */
 extern int
 force_symlinkat (char const *srcname, int dstdir, char const *dstname,
-                 bool force)
+                 bool force, int symlinkat_errno)
 {
-  int r = symlinkat (srcname, dstdir, dstname);
-  if (!force || r == 0 || errno != EEXIST)
-    return r;
+  if (symlinkat_errno < 0)
+    symlinkat_errno = symlinkat (srcname, dstdir, dstname) == 0 ? 0 : errno;
+  if (!force || symlinkat_errno != EEXIST)
+    return symlinkat_errno;
 
   char buf[smallsize];
   char *dsttmp = samedir_template (dstname, buf);
   if (!dsttmp)
-    return -1;
+    return errno;
   struct symlink_arg arg = { srcname, dstdir };
   int err;
 
@@ -170,13 +175,10 @@ force_symlinkat (char const *srcname, int dstdir, char const *dstname,
     {
       /* Don't worry about renameat being a no-op, since DSTTMP is
          newly created.  */
-      err = 0;
+      err = -1;
     }
 
   if (dsttmp != buf)
     free (dsttmp);
-  if (!err)
-    return 1;
-  errno = err;
-  return -1;
+  return err;
 }
diff --git a/src/force-link.h b/src/force-link.h
index 7ea3817de..595c93f1a 100644
--- a/src/force-link.h
+++ b/src/force-link.h
@@ -1,2 +1,2 @@
-extern int force_linkat (int, char const *, int, char const *, int, bool);
-extern int force_symlinkat (char const *, int, char const *, bool);
+extern int force_linkat (int, char const *, int, char const *, int, bool, int);
+extern int force_symlinkat (char const *, int, char const *, bool, int);
diff --git a/src/ln.c b/src/ln.c
index 7bbd42f71..9f197a4b7 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -30,8 +30,10 @@
 #include "force-link.h"
 #include "hash.h"
 #include "hash-triple.h"
+#include "priv-set.h"
 #include "relpath.h"
 #include "same.h"
+#include "unlinkdir.h"
 #include "yesno.h"
 #include "canonicalize.h"
 
@@ -69,6 +71,9 @@ static bool verbose;
    directories on most existing systems (Solaris being an exception).  */
 static bool hard_dir_link;
 
+/* If true, watch out for creating or removing hard links to directories.  */
+static bool beware_hard_dir_link;
+
 /* If nonzero, and the specified destination is a symbolic link to a
    directory, treat it just as if it were a directory.  Otherwise, the
    command 'ln --force --no-dereference file symlink-to-dir' deletes
@@ -113,6 +118,14 @@ errno_nonexisting (int err)
   return err == ENOENT || err == ENAMETOOLONG || err == ENOTDIR || err == ELOOP;
 }
 
+/* Return an errno value for a system call that returned STATUS.
+   This is zero if STATUS is zero, and is errno otherwise.  */
+
+static int
+errnoize (int status)
+{
+  return status < 0 ? errno : 0;
+}
 
 /* FILE is the last operand of this command.  Return true if FILE is a
    directory.  But report an error if there is a problem accessing FILE,
@@ -126,9 +139,8 @@ target_directory_operand (char const *file)
   size_t blen = strlen (b);
   bool looks_like_a_dir = (blen == 0 || ISSLASH (b[blen - 1]));
   struct stat st;
-  int stat_result =
-    (dereference_dest_dir_symlinks ? stat (file, &st) : lstat (file, &st));
-  int err = (stat_result == 0 ? 0 : errno);
+  int flags = dereference_dest_dir_symlinks ? 0 : AT_SYMLINK_NOFOLLOW;
+  int err = errnoize (fstatat (AT_FDCWD, file, &st, flags));
   bool is_a_dir = !err && S_ISDIR (st.st_mode);
   if (err && ! errno_nonexisting (errno))
     die (EXIT_FAILURE, err, _("failed to access %s"), quoteaf (file));
@@ -171,162 +183,181 @@ convert_abs_rel (const char *from, const char *target)
   return relative_from ? relative_from : xstrdup (from);
 }
 
+/* Link SOURCE to DEST atomically.  Return 0 if successful, a positive
+   errno value on failure, and -1 if an atomic link cannot be done.
+   This handles the common case where DEST does not already exist and
+   -r is not specified.  */
+
+static int
+atomic_link (char const *source, char const *dest)
+{
+  return (symbolic_link
+          ? (relative ? -1 : errnoize (symlink (source, dest)))
+          : beware_hard_dir_link
+          ? -1
+          : errnoize (linkat (AT_FDCWD, source, AT_FDCWD, dest,
+                              logical ? AT_SYMLINK_FOLLOW : 0)));
+}
+
 /* Make a link DEST to the (usually) existing file SOURCE.
    Symbolic links to nonexistent files are allowed.
+   LINK_ERRNO is zero if the link has already been made,
+   positive if attempting the link failed with errno == LINK_ERRNO,
+   -1 if no attempt has been made to create the link.
    Return true if successful.  */
 
 static bool
-do_link (const char *source, const char *dest)
+do_link (const char *source, const char *dest, int link_errno)
 {
   struct stat source_stats;
-  struct stat dest_stats;
+  int source_status = 1;
   char *dest_backup = NULL;
   char *rel_source = NULL;
-  bool dest_lstat_ok = false;
-  bool source_is_dir = false;
+  int nofollow_flag = logical ? 0 : AT_SYMLINK_NOFOLLOW;
+  if (link_errno < 0)
+    link_errno = atomic_link (source, dest);
 
-  if (!symbolic_link)
+  /* Get SOURCE_STATS if later code will need it, if only for sharper
+     diagnostics.  */
+  if ((link_errno || dest_set) && !symbolic_link)
     {
-       /* Which stat to use depends on whether linkat will follow the
-          symlink.  We can't use the shorter
-          (logical?stat:lstat) (source, &source_stats)
-          since stat might be a function-like macro.  */
-      if ((logical ? stat (source, &source_stats)
-           : lstat (source, &source_stats))
-          != 0)
+      source_status = fstatat (AT_FDCWD, source, &source_stats, nofollow_flag);
+      if (source_status != 0)
         {
           error (0, errno, _("failed to access %s"), quoteaf (source));
           return false;
         }
-
-      if (S_ISDIR (source_stats.st_mode))
-        {
-          source_is_dir = true;
-          if (! hard_dir_link)
-            {
-              error (0, 0, _("%s: hard link not allowed for directory"),
-                     quotef (source));
-              return false;
-            }
-        }
     }
 
-  if (remove_existing_files || interactive || backup_type != no_backups)
+  if (link_errno)
     {
-      dest_lstat_ok = (lstat (dest, &dest_stats) == 0);
-      if (!dest_lstat_ok && errno != ENOENT)
+      if (!symbolic_link && !hard_dir_link && S_ISDIR (source_stats.st_mode))
         {
-          error (0, errno, _("failed to access %s"), quoteaf (dest));
+          error (0, 0, _("%s: hard link not allowed for directory"),
+                 quotef (source));
           return false;
         }
-    }
-
-  /* If the current target was created as a hard link to another
-     source file, then refuse to unlink it.  */
-  if (dest_lstat_ok
-      && dest_set != NULL
-      && seen_file (dest_set, dest, &dest_stats))
-    {
-      error (0, 0,
-             _("will not overwrite just-created %s with %s"),
-             quoteaf_n (0, dest), quoteaf_n (1, source));
-      return false;
-    }
-
-  /* If --force (-f) has been specified without --backup, then before
-     making a link ln must remove the destination file if it exists.
-     (with --backup, it just renames any existing destination file)
-     But if the source and destination are the same, don't remove
-     anything and fail right here.  */
-  if ((remove_existing_files
-       /* Ensure that "ln --backup f f" fails here, with the
-          "... same file" diagnostic, below.  Otherwise, subsequent
-          code would give a misleading "file not found" diagnostic.
-          This case is different than the others handled here, since
-          the command in question doesn't use --force.  */
-       || (!symbolic_link && backup_type != no_backups))
-      && dest_lstat_ok
-      /* Allow 'ln -sf --backup k k' to succeed in creating the
-         self-referential symlink, but don't allow the hard-linking
-         equivalent: 'ln -f k k' (with or without --backup) to get
-         beyond this point, because the error message you'd get is
-         misleading.  */
-      && (backup_type == no_backups || !symbolic_link)
-      && (!symbolic_link || stat (source, &source_stats) == 0)
-      && SAME_INODE (source_stats, dest_stats)
-      /* The following detects whether removing DEST will also remove
-         SOURCE.  If the file has only one link then both are surely
-         the same link.  Otherwise check whether they point to the same
-         name in the same directory.  */
-      && (source_stats.st_nlink == 1 || same_name (source, dest)))
-    {
-      error (0, 0, _("%s and %s are the same file"),
-             quoteaf_n (0, source), quoteaf_n (1, dest));
-      return false;
-    }
 
-  if (dest_lstat_ok)
-    {
-      if (S_ISDIR (dest_stats.st_mode))
-        {
-          error (0, 0, _("%s: cannot overwrite directory"), quotef (dest));
-          return false;
-        }
-      if (interactive)
-        {
-          fprintf (stderr, _("%s: replace %s? "), program_name, quoteaf (dest));
-          if (!yesno ())
-            return true;
-          remove_existing_files = true;
-        }
+      if (relative)
+        source = rel_source = convert_abs_rel (source, dest);
 
-      if (backup_type != no_backups)
+      bool force = (remove_existing_files || interactive
+                    || backup_type != no_backups);
+      if (force)
         {
-          dest_backup = find_backup_file_name (dest, backup_type);
-          if (rename (dest, dest_backup) != 0)
+          struct stat dest_stats;
+          if (lstat (dest, &dest_stats) != 0)
             {
-              int rename_errno = errno;
-              free (dest_backup);
-              dest_backup = NULL;
-              if (rename_errno != ENOENT)
+              if (errno != ENOENT)
                 {
-                  error (0, rename_errno, _("cannot backup %s"),
-                         quoteaf (dest));
+                  error (0, errno, _("failed to access %s"), quoteaf (dest));
                   return false;
                 }
+              force = false;
+            }
+          else if (S_ISDIR (dest_stats.st_mode))
+            {
+              error (0, 0, _("%s: cannot overwrite directory"), quotef (dest));
+              return false;
+            }
+          else if (seen_file (dest_set, dest, &dest_stats))
+            {
+              /* The current target was created as a hard link to another
+                 source file.  */
+              error (0, 0,
+                     _("will not overwrite just-created %s with %s"),
+                     quoteaf_n (0, dest), quoteaf_n (1, source));
+              return false;
+            }
+          else
+            {
+              /* Beware removing DEST if it is the same directory entry as
+                 SOURCE, because in that case removing DEST can cause the
+                 subsequent link creation either to fail (for hard links), or
+                 to replace a non-symlink DEST with a self-loop (for symbolic
+                 links) which loses the contents of DEST.  So, when backing
+                 up, worry about creating hard links (since the backups cover
+                 the symlink case); otherwise, worry about about -f.  */
+              if (backup_type != no_backups
+                  ? !symbolic_link
+                  : remove_existing_files)
+                {
+                  /* Detect whether removing DEST would also remove SOURCE.
+                     If the file has only one link then both are surely the
+                     same directory entry.  Otherwise check whether they point
+                     to the same name in the same directory.  */
+                  if (source_status != 0)
+                    source_status = stat (source, &source_stats);
+                  if (source_status == 0
+                      && SAME_INODE (source_stats, dest_stats)
+                      && (source_stats.st_nlink == 1
+                          || same_name (source, dest)))
+                    {
+                      error (0, 0, _("%s and %s are the same file"),
+                             quoteaf_n (0, source), quoteaf_n (1, dest));
+                      return false;
+                    }
+                }
+
+              if (link_errno < 0 || link_errno == EEXIST)
+                {
+                  if (interactive)
+                    {
+                      fprintf (stderr, _("%s: replace %s? "),
+                               program_name, quoteaf (dest));
+                      if (!yesno ())
+                        return true;
+                    }
+
+                  if (backup_type != no_backups)
+                    {
+                      dest_backup = find_backup_file_name (dest, backup_type);
+                      if (rename (dest, dest_backup) != 0)
+                        {
+                          int rename_errno = errno;
+                          free (dest_backup);
+                          dest_backup = NULL;
+                          if (rename_errno != ENOENT)
+                           {
+                              error (0, rename_errno, _("cannot backup %s"),
+                                     quoteaf (dest));
+                              return false;
+                            }
+                          force = false;
+                        }
+                    }
+                }
             }
         }
+
+      /* If the attempt to create a link fails and we are removing or
+         backing up destinations, unlink the destination and try again.
+
+         On the surface, POSIX states that 'ln -f A B' unlinks B before trying
+         to link A to B.  But strictly following this has the counterintuitive
+         effect of losing the contents of B if A does not exist.  Fortunately,
+         POSIX 2008 clarified that an application is free to fail early if it
+         can prove that continuing onwards cannot succeed, so we can try to
+         link A to B before blindly unlinking B, thus sometimes attempting to
+         link a second time during a successful 'ln -f A B'.
+
+         Try to unlink DEST even if we may have backed it up successfully.
+         In some unusual cases (when DEST and DEST_BACKUP are hard-links
+         that refer to the same file), rename succeeds and DEST remains.
+         If we didn't remove DEST in that case, the subsequent symlink or
+         link call would fail.  */
+      link_errno
+        = (symbolic_link
+           ? force_symlinkat (source, AT_FDCWD, dest, force, link_errno)
+           : force_linkat (AT_FDCWD, source, AT_FDCWD, dest,
+                           logical ? AT_SYMLINK_FOLLOW : 0,
+                           force, link_errno));
+      /* Until now, link_errno < 0 meant the link has not been tried.
+         From here on, link_errno < 0 means the link worked but
+         required removing the destination first.  */
     }
 
-  if (relative)
-    source = rel_source = convert_abs_rel (source, dest);
-
-  /* If the attempt to create a link fails and we are removing or
-     backing up destinations, unlink the destination and try again.
-
-     On the surface, POSIX describes an algorithm that states that
-     'ln -f A B' will call unlink() on B before ever attempting
-     link() on A.  But strictly following this has the counterintuitive
-     effect of losing the contents of B, if A does not exist.
-     Fortunately, POSIX 2008 clarified that an application is free
-     to fail early if it can prove that continuing onwards cannot
-     succeed, so we are justified in trying link() before blindly
-     removing B, thus sometimes calling link() a second time during
-     a successful 'ln -f A B'.
-
-     Try to unlink DEST even if we may have backed it up successfully.
-     In some unusual cases (when DEST and DEST_BACKUP are hard-links
-     that refer to the same file), rename succeeds and DEST remains.
-     If we didn't remove DEST in that case, the subsequent symlink or link
-     call would fail.  */
-  bool ok_to_remove = remove_existing_files || dest_backup;
-  bool ok = 0 <= (symbolic_link
-                  ? force_symlinkat (source, AT_FDCWD, dest, ok_to_remove)
-                  : force_linkat (AT_FDCWD, source, AT_FDCWD, dest,
-                                  logical ? AT_SYMLINK_FOLLOW : 0,
-                                  ok_to_remove));
-
-  if (ok)
+  if (link_errno <= 0)
     {
       /* Right after creating a hard link, do this: (note dest name and
          source_stats, which are also the just-linked-destinations stats) */
@@ -343,15 +374,15 @@ do_link (const char *source, const char *dest)
     }
   else
     {
-      error (0, errno,
+      error (0, link_errno,
              (symbolic_link
-              ? (errno != ENAMETOOLONG && *source
+              ? (link_errno != ENAMETOOLONG && *source
                  ? _("failed to create symbolic link %s")
                  : _("failed to create symbolic link %s -> %s"))
-              : (errno == EMLINK && !source_is_dir
+              : (link_errno == EMLINK
                  ? _("failed to create hard link to %.0s%s")
-                 : (errno == EDQUOT || errno == EEXIST || errno == ENOSPC
-                    || errno == EROFS)
+                 : (link_errno == EDQUOT || link_errno == EEXIST
+                    || link_errno == ENOSPC || link_errno == EROFS)
                  ? _("failed to create hard link %s")
                  : _("failed to create hard link %s => %s"))),
              quoteaf_n (0, dest), quoteaf_n (1, source));
@@ -365,7 +396,7 @@ do_link (const char *source, const char *dest)
 
   free (dest_backup);
   free (rel_source);
-  return ok;
+  return link_errno <= 0;
 }
 
 void
@@ -444,6 +475,7 @@ main (int argc, char **argv)
   bool no_target_directory = false;
   int n_files;
   char **file;
+  int link_errno = -1;
 
   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -535,6 +567,15 @@ main (int argc, char **argv)
       usage (EXIT_FAILURE);
     }
 
+  if (relative && !symbolic_link)
+    die (EXIT_FAILURE, 0, _("cannot do --relative without --symbolic"));
+
+  if (!hard_dir_link)
+    {
+      priv_set_remove_linkdir ();
+      beware_hard_dir_link = !cannot_unlink_dir ();
+    }
+
   if (no_target_directory)
     {
       if (target_directory)
@@ -556,11 +597,17 @@ main (int argc, char **argv)
     {
       if (n_files < 2)
         target_directory = ".";
-      else if (2 <= n_files && target_directory_operand (file[n_files - 1]))
-        target_directory = file[--n_files];
-      else if (2 < n_files)
-        die (EXIT_FAILURE, 0, _("target %s is not a directory"),
-             quoteaf (file[n_files - 1]));
+      else
+        {
+          if (n_files == 2)
+            link_errno = atomic_link (file[0], file[1]);
+          if ((link_errno < 0 || link_errno == EEXIST || link_errno == ENOTDIR)
+              && target_directory_operand (file[n_files - 1]))
+            target_directory = file[--n_files];
+          else if (2 < n_files)
+            die (EXIT_FAILURE, 0, _("target %s is not a directory"),
+                 quoteaf (file[n_files - 1]));
+        }
     }
 
   backup_type = (make_backups
@@ -568,12 +615,6 @@ main (int argc, char **argv)
                  : no_backups);
   set_simple_backup_suffix (backup_suffix);
 
-  if (relative && !symbolic_link)
-    {
-        die (EXIT_FAILURE, 0,
-             _("cannot do --relative without --symbolic"));
-    }
-
 
   if (target_directory)
     {
@@ -607,12 +648,12 @@ main (int argc, char **argv)
                                          last_component (file[i]),
                                          &dest_base);
           strip_trailing_slashes (dest_base);
-          ok &= do_link (file[i], dest);
+          ok &= do_link (file[i], dest, -1);
           free (dest);
         }
     }
   else
-    ok = do_link (file[0], file[1]);
+    ok = do_link (file[0], file[1], link_errno);
 
   return ok ? EXIT_SUCCESS : EXIT_FAILURE;
 }
-- 
2.17.2





bug closed, send any further explanations to 33096 <at> debbugs.gnu.org and Paul Eggert <eggert <at> cs.ucla.edu> Request was from Paul Eggert <eggert <at> cs.ucla.edu> to control <at> debbugs.gnu.org. (Fri, 19 Oct 2018 19:48:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 17 Nov 2018 12:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 275 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.