Do not diagnose same xattr file twice

* src/extract.c (set_xattr): Simplify, by having it do only
the mknodat and xattrs_xattrs_set, rather than also
trying to recover from failure.  Caller simplified too.
* tests/xattr07.at (xattrs: xattrs and --skip-old-files):
Adjust test to match fixed behavior.

diff --git a/src/extract.c b/src/extract.c
index c1f5731bad2ebd9a1232f25fb8da2a71b52c9077..7696d5e462dea2f4c58d4b8460542c4edb325d6b 100644 (file)
--- a/src/extract.c
+++ b/src/extract.c
@@ -903,42 +903,21 @@ maybe_recoverable (char *file_name, bool regular, bool *interdir_made)
    in advance dramatically improves the following  performance of reading and
    writing a file).  If not restoring permissions, invert the INVERT_PERMISSIONS
    bits from the file's current permissions.  TYPEFLAG specifies the type of the
-   file.  Returns non-zero when error occurs (while un-available xattrs is not
-   an error, rather no-op).  Non-zero FILE_CREATED indicates set_xattr has
-   created the file. */
+   file.  Return a negative number (setting errno) on failure, zero if
+   successful but FILE_NAME was not created (e.g., xattrs not
+   available), and a positive number if FILE_NAME was created.  */
 static int
 set_xattr (char const *file_name, struct tar_stat_info const *st,
-           mode_t invert_permissions, char typeflag, int *file_created)
+           mode_t mode, char typeflag)
 {
 #ifdef HAVE_XATTRS
-  bool interdir_made = false;
-
   if ((xattrs_option > 0) && st->xattr_map.xm_size)
     {
-      mode_t mode = current_stat_info.stat.st_mode & MODE_RWX & ~ current_umask;
-
-      for (;;)
-        {
-          if (!mknodat (chdir_fd, file_name, mode ^ invert_permissions, 0))
-            {
-              /* Successfully created file */
-              xattrs_xattrs_set (st, file_name, typeflag, 0);
-              *file_created = 1;
-              return 0;
-            }
-
-          switch (maybe_recoverable ((char *)file_name, false, &interdir_made))
-            {
-              case RECOVER_OK:
-                continue;
-              case RECOVER_NO:
-                skip_member ();
-                open_error (file_name);
-                return 1;
-              case RECOVER_SKIP:
-                return 0;
-            }
-        }
+      int r = mknodat (chdir_fd, file_name, mode, 0);
+      if (r < 0)
+       return r;
+      xattrs_xattrs_set (st, file_name, typeflag, 0);
+      return 1;
     }
 #endif
 
@@ -1266,9 +1245,6 @@ extract_file (char *file_name, int typeflag)
   bool interdir_made = false;
   mode_t mode = (current_stat_info.stat.st_mode & MODE_RWX
                 & ~ (0 < same_owner_option ? S_IRWXG | S_IRWXO : 0));
-  mode_t invert_permissions
-    = ((0 < same_owner_option ? mode & (S_IRWXG | S_IRWXO) : 0)
-       | ((mode & S_IWUSR) ^ S_IWUSR));
   mode_t current_mode = 0;
   mode_t current_mode_mask = 0;
 
@@ -1285,15 +1261,14 @@ extract_file (char *file_name, int typeflag)
     }
   else
     {
-      int file_created = 0;
-      if (set_xattr (file_name, &current_stat_info, invert_permissions,
-                     typeflag, &file_created))
-        return 1;
-
-      while ((fd = open_output_file (file_name, typeflag, mode,
-                                     file_created, &current_mode,
-                                     &current_mode_mask))
-            < 0)
+      int file_created;
+      while (((file_created = set_xattr (file_name, &current_stat_info,
+                                        mode | S_IWUSR, typeflag))
+             < 0)
+            || ((fd = open_output_file (file_name, typeflag, mode,
+                                        file_created, &current_mode,
+                                        &current_mode_mask))
+                < 0))
        {
          int recover = maybe_recoverable (file_name, true, &interdir_made);
          if (recover != RECOVER_OK)

diff --git a/tests/xattr07.at b/tests/xattr07.at
index dcc3aabe4d9694fc725fbc03e02c41883687adcd..fa6797d8acd6b7e15186239f0f47bd28456c6559 100644 (file)
--- a/tests/xattr07.at
+++ b/tests/xattr07.at
@@ -62,8 +62,6 @@ user.test="OurFileValue2"
 user.test="OurFileValue2"
 ], [tar: dir: skipping existing file
 tar: dir/file: skipping existing file
-tar: dir/file: skipping existing file
-tar: dir/file2: skipping existing file
 tar: dir/file2: skipping existing file
 tar: dir/file: Cannot open: File exists
 tar: dir/file2: Cannot open: File exists