[Bug-tar] [PATCH] tar: fix bug with -C and delayed setting of metadata

[Paul Eggert]

I found a bug by code inspection, and installed the following
bug fix and test case for the bug.

>From 6cc62eb8cd0aa36d456ac5435dd152cb2c3cd459 Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden>
Date: Wed, 25 Aug 2010 17:09:17 -0700
Subject: [PATCH] tar: fix bug with -C and delayed setting of metadata

* src/common.h (chdir_current): New decl.
* src/extract.c (struct delayed_set_stat, struct delayed_link):
New member change_dir.
(delay_set_stat, create_placeholder_file): Set it.
(apply_nonancestor_delayed_set_stat, apply_delayed_links): Use it.
(extract_link): Check that the links are all relative to the same
directory.
(extract_archive): Restore the current directory after
apply_nonancestor_delayed_set_stat has possibly changed it.
* src/misc.c (chdir_current): New external var; this used to
be the private static variable 'previous' inside chdir_dir.
All uses changed.
* tests/Makefile.am (TESTSUITE_AT): New test extrac10.at.
* tests/extrac10.at: New file.
* tests/testsuite.at: Include it.
---
 src/common.h       |    1 +
 src/extract.c      |   21 ++++++++++++++++++---
 src/misc.c         |   11 ++++++-----
 tests/Makefile.am  |    1 +
 tests/extrac10.at  |   44 ++++++++++++++++++++++++++++++++++++++++++++
 tests/testsuite.at |    1 +
 6 files changed, 71 insertions(+), 8 deletions(-)
 create mode 100644 tests/extrac10.at

diff --git a/src/common.h b/src/common.h
index f6f3844..e5e619d 100644
--- a/src/common.h
+++ b/src/common.h
@@ -603,6 +603,7 @@ void undo_last_backup (void);
 
 int deref_stat (bool deref, char const *name, struct stat *buf);
 
+extern int chdir_current;
 int chdir_arg (char const *dir);
 void chdir_do (int dir);
 int chdir_count (void);
diff --git a/src/extract.c b/src/extract.c
index dad7746..5b12ed1 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -69,6 +69,7 @@ struct delayed_set_stat
     mode_t invert_permissions;
     enum permstatus permstatus;
     bool after_links;
+    int change_dir;
     char file_name[1];
   };
 
@@ -94,6 +95,9 @@ struct delayed_link
     uid_t uid;
     gid_t gid;
 
+    /* The directory that the sources and target are relative to.  */
+    int change_dir;
+
     /* A list of sources for this link.  The sources are all to be
        hard-linked together.  */
     struct string_list *sources;
@@ -373,6 +377,7 @@ delay_set_stat (char const *file_name, struct tar_stat_info 
const *st,
   data->invert_permissions = invert_permissions;
   data->permstatus = permstatus;
   data->after_links = 0;
+  data->change_dir = chdir_current;
   strcpy (data->file_name, file_name);
   delayed_set_stat_head = data;
 }
@@ -606,6 +611,8 @@ apply_nonancestor_delayed_set_stat (char const *file_name, 
bool after_links)
              && memcmp (file_name, data->file_name, data->file_name_len) == 0))
        break;
 
+      chdir_do (data->change_dir);
+
       if (check_for_renamed_directories)
        {
          cur_info = &st;
@@ -933,6 +940,7 @@ create_placeholder_file (char *file_name, bool is_symlink, 
bool *interdir_made)
          p->uid = current_stat_info.stat.st_uid;
          p->gid = current_stat_info.stat.st_gid;
        }
+      p->change_dir = chdir_current;
       p->sources = xmalloc (offsetof (struct string_list, string)
                            + strlen (file_name) + 1);
       p->sources->next = 0;
@@ -990,7 +998,8 @@ extract_link (char *file_name, int typeflag)
          struct delayed_link *ds = delayed_link_head;
          if (ds && lstat (link_name, &st1) == 0)
            for (; ds; ds = ds->next)
-             if (ds->dev == st1.st_dev
+             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)
                {
@@ -1297,7 +1306,11 @@ extract_archive (void)
      it is an incremental archive.
      (see NOTICE in the comment to delay_set_stat above) */
   if (!delay_directory_restore_option)
-    apply_nonancestor_delayed_set_stat (current_stat_info.file_name, 0);
+    {
+      int dir = chdir_current;
+      apply_nonancestor_delayed_set_stat (current_stat_info.file_name, 0);
+      chdir_do (dir);
+    }
 
   /* Take a safety backup of a previously existing file.  */
 
@@ -1327,7 +1340,7 @@ extract_archive (void)
 
 }
 
-/* Extract the symbolic links whose final extraction were delayed.  */
+/* Extract the links whose final extraction were delayed.  */
 static void
 apply_delayed_links (void)
 {
@@ -1338,6 +1351,8 @@ apply_delayed_links (void)
       struct string_list *sources = ds->sources;
       char const *valid_source = 0;
 
+      chdir_do (ds->change_dir);
+
       for (sources = ds->sources; sources; sources = sources->next)
        {
          char const *source = sources->string;
diff --git a/src/misc.c b/src/misc.c
index 4bae75e..6f67887 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -712,16 +712,17 @@ chdir_arg (char const *dir)
   return wd_count++;
 }
 
+/* Index of current directory.  */
+int chdir_current;
+
 /* Change to directory I.  If I is 0, change to the initial working
    directory; otherwise, I must be a value returned by chdir_arg.  */
 void
 chdir_do (int i)
 {
-  static int previous;
-
-  if (previous != i)
+  if (chdir_current != i)
     {
-      struct wd *prev = &wd[previous];
+      struct wd *prev = &wd[chdir_current];
       struct wd *curr = &wd[i];
 
       if (prev->err < 0)
@@ -766,7 +767,7 @@ chdir_do (int i)
            chdir_fatal (curr->name);
        }
 
-      previous = i;
+      chdir_current = i;
     }
 }
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1026570..4b70000 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -76,6 +76,7 @@ TESTSUITE_AT = \
  extrac07.at\
  extrac08.at\
  extrac09.at\
+ extrac10.at\
  filerem01.at\
  filerem02.at\
  gzip.at\
diff --git a/tests/extrac10.at b/tests/extrac10.at
new file mode 100644
index 0000000..5a53f64
--- /dev/null
+++ b/tests/extrac10.at
@@ -0,0 +1,44 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+
+# Test suite for GNU tar.
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# written by Paul Eggert
+
+# Check that delayed setting of directory metadata does not collide
+# with the -C option.  When setting a directory's permissions, time
+# stamps, etc., tar should apply the -C option that was in effect when
+# the directory was extracted, not the -C option that happens to be in
+# effect when the metadata are later set.
+
+AT_SETUP([-C and delayed setting of metadata])
+AT_KEYWORDS([extract extrac10])
+
+AT_TAR_CHECK([
+mkdir d x x/y
+echo foo >d/d1
+echo bar >e
+
+tar -cf archive.tar d e &&
+tar -xf archive.tar -C x d -C y e &&
+diff -r d x/d &&
+diff e x/y/e
+],
+[0],
+[],
+[],[],[],[gnu])
+
+AT_CLEANUP
diff --git a/tests/testsuite.at b/tests/testsuite.at
index e69031c..8866ed0 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -148,6 +148,7 @@ m4_include([extrac06.at])
 m4_include([extrac07.at])
 m4_include([extrac08.at])
 m4_include([extrac09.at])
+m4_include([extrac10.at])
 
 m4_include([label01.at])
 m4_include([label02.at])
-- 
1.7.2