Re: [Bug-tar] gnu tar fails to restore hard-linked symlinks

[Paul Eggert]

I was about to respond with an email saying "works for me"
(both Fedora 14 and Solaris 10) when I tried it a few more times and
got it to fail.  It's a timing-dependent bug.  Could you please try
the following patch?  It works for me, but since it is a race
condition it'd be nice if you could try it on your hosts too.
Thanks (and thanks for reporting this bug!).

I've pushed this as commit 189e4364741e927216a254b8a51d96af2d954df8.

tar: use birthtime rather than ctime when checking identity
Without this fix, tar would sometimes mishandle the extraction of
hard links to symbolic links.  Problem reported in
<http://lists.gnu.org/archive/html/bug-tar/2011-06/msg00000.html>
by Andreas Stolcke.
* src/extract.c (struct delayed_link): Change ctime member to birthtime.
(same_birthtime): New function.
(create_placeholder_file, extract_link, apply_delayed_links):
Use it to compare birthtimes (when available) instead of ctime.
diff --git a/src/extract.c b/src/extract.c
index aaea56e..ecbde1a 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -110,12 +110,15 @@ struct delayed_link
     /* The next delayed link in the list.  */
     struct delayed_link *next;

-    /* The device, inode number and ctime of the placeholder.  Use
-       ctime, not mtime, to make false matches less likely if some
-       other process removes the placeholder.  */
+    /* The device, inode number and birthtime of the placeholder.
+       birthtime.tv_nsec is negative if the birthtime is not available.
+       Don't use mtime as this would allow for false matches if some
+       other process removes the placeholder.  Don't use ctime as
+       this would cause race conditions and other screwups, e.g.,
+       when restoring hard-linked symlinks.  */
     dev_t dev;
     ino_t ino;
-    struct timespec ctime;
+    struct timespec birthtime;

     /* True if the link is symbolic.  */
     bool is_symlink;
@@ -268,6 +271,15 @@ set_mode (char const *file_name,
     }
 }

+/* Return true if A and B are the same birthtimes.
+   Unavailable birthtimes, which have negative tv_nsec members,
+   all compare equal to each other.  */
+static bool
+same_birthtime (struct timespec a, struct timespec b)
+{
+  return (a.tv_nsec == b.tv_nsec && (a.tv_nsec < 0 || a.tv_sec == b.tv_sec));
+}
+
 /* Check time after successfully setting FILE_NAME's time stamp to T.  */
 static void
 check_time (char const *file_name, struct timespec t)
@@ -1076,7 +1088,7 @@ create_placeholder_file (char *file_name, bool 
is_symlink, bool *interdir_made)
       delayed_link_head = p;
       p->dev = st.st_dev;
       p->ino = st.st_ino;
-      p->ctime = get_stat_ctime (&st);
+      p->birthtime = get_stat_birthtime (&st);
       p->is_symlink = is_symlink;
       if (is_symlink)
        {
@@ -1134,7 +1146,7 @@ extract_link (char *file_name, int typeflag)
              if (ds->change_dir == chdir_current
                  && ds->dev == st1.st_dev
                  && ds->ino == st1.st_ino
-                 && timespec_cmp (ds->ctime, get_stat_ctime (&st1)) == 0)
+                 && same_birthtime (ds->birthtime, get_stat_birthtime (&st1)))
                {
                  struct string_list *p =  xmalloc (offsetof (struct 
string_list, string)
                                                    + strlen (file_name) + 1);
@@ -1500,7 +1512,7 @@ apply_delayed_links (void)
          if (fstatat (chdir_fd, source, &st, AT_SYMLINK_NOFOLLOW) == 0
              && st.st_dev == ds->dev
              && st.st_ino == ds->ino
-             && timespec_cmp (get_stat_ctime (&st), ds->ctime) == 0)
+             && same_birthtime (get_stat_birthtime (&st), ds->birthtime))
            {
              /* Unlink the placeholder, then create a hard link if possible,
                 a symbolic link otherwise.  */