Add additional error handling to safe_rename()

It is apparently possible for link() to return an error but the link
to still be created.  Add a double check for that case.  If the files
match, unlink the src and return success.

diff --git a/mutt/file.c b/mutt/file.c
index 91da3e1a7d088d0c92605fa206c702366c305be4..a7c6688cf56471a8d708e46ae32c441e6410c636 100644 (file)
--- a/mutt/file.c
+++ b/mutt/file.c
@@ -338,12 +338,36 @@ int mutt_file_symlink(const char *oldpath, const char *newpath)
 int mutt_file_safe_rename(const char *src, const char *target)
 {
   struct stat ssb, tsb;
+  int link_errno;
 
   if (!src || !target)
     return -1;
 
   if (link(src, target) != 0)
   {
+    link_errno = errno;
+
+    /*
+     * It is historically documented that link can return -1 if NFS
+     * dies after creating the link.  In that case, we are supposed
+     * to use stat to check if the link was created.
+     *
+     * Derek Martin notes that some implementations of link() follow a
+     * source symlink.  It might be more correct to use stat() on src.
+     * I am not doing so to minimize changes in behavior: the function
+     * used lstat() further below for 20 years without issue, and I
+     * believe was never intended to be used on a src symlink.
+     */
+    if ((lstat(src, &ssb) == 0) && (lstat(target, &tsb) == 0) &&
+        (compare_stat(&ssb, &tsb) == 0))
+    {
+      mutt_debug(1, "link (%s, %s) reported failure: %s (%d) but actually succeded\n",
+                 src, target, strerror(errno), errno);
+      goto success;
+    }
+
+    errno = link_errno;
+
     /* Coda does not allow cross-directory links, but tells
      * us it's a cross-filesystem linking attempt.
      *
@@ -412,6 +436,7 @@ int mutt_file_safe_rename(const char *src, const char *target)
   }
 #endif
 
+success:
   /* Unlink the original link.
    * Should we really ignore the return value here? XXX */
   if (unlink(src) == -1)