From 6bd6bf244fb1b6bc07eb9cd1cd0bfdfd76576219 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Fri, 11 Nov 2016 12:30:35 +0200 Subject: [PATCH] don't set xattrs when --skip-old-files is used * src/extract.c (set_xattr): Properly handle maybe_recoverable() output. Throw warnings to not complicate caller. (extract_file): Don't handle set_xattr's error. * tests/xattr07.at: New testcase. * tests/Makefile.am: Mention new testcase. * tests/testsuite.at: Likewise. * THANKS: Dawid. Upstream commits 597b0ae509 and ca9399d4e. --- THANKS | 1 + src/extract.c | 41 +++++++++++++++++++----------- tests/Makefile.am | 1 + tests/testsuite.at | 1 + tests/xattr07.at | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 15 deletions(-) create mode 100644 tests/xattr07.at diff --git a/THANKS b/THANKS index 3dda4cc..559f240 100644 --- a/THANKS +++ b/THANKS @@ -138,6 +138,7 @@ David Nugent davidn@blaze.net.au David Shaw david.shaw@alcatel.com.au David Steiner dsteiner@ispa.uni-osnabrueck.de David Taylor taylor@think.com +Dawid dpc@dpc.pw Dean Gaudet dgaudet@watdragon.uwaterloo.ca Demizu Noritoshi nori-d@is.aist-nara.ac.jp Denis Excoffier denis.excoffier@free.fr diff --git a/src/extract.c b/src/extract.c index f982433..67885d7 100644 --- a/src/extract.c +++ b/src/extract.c @@ -795,13 +795,13 @@ 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. FILE_CREATED indicates set_xattr has created the file */ + 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. */ static int set_xattr (char const *file_name, struct tar_stat_info const *st, mode_t invert_permissions, char typeflag, int *file_created) { - int status = 0; - #ifdef HAVE_XATTRS bool interdir_made = false; @@ -809,17 +809,32 @@ set_xattr (char const *file_name, struct tar_stat_info const *st, { mode_t mode = current_stat_info.stat.st_mode & MODE_RWX & ~ current_umask; - do - status = mknodat (chdir_fd, file_name, mode ^ invert_permissions, 0); - while (status && maybe_recoverable ((char *)file_name, false, - &interdir_made)); + 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; + } - xattrs_xattrs_set (st, file_name, typeflag, 0); - *file_created = 1; + 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; + } + } } #endif - return(status); + return 0; } /* Fix the statuses of all directories whose statuses need fixing, and @@ -1136,11 +1151,7 @@ extract_file (char *file_name, int typeflag) int file_created = 0; if (set_xattr (file_name, ¤t_stat_info, invert_permissions, typeflag, &file_created)) - { - skip_member (); - open_error (file_name); - return 1; - } + return 1; while ((fd = open_output_file (file_name, typeflag, mode, file_created, ¤t_mode, diff --git a/tests/Makefile.am b/tests/Makefile.am index abc17a4..4f3b918 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -243,6 +243,7 @@ TESTSUITE_AT = \ xattr04.at\ xattr05.at\ xattr06.at\ + xattr07.at\ acls01.at\ acls02.at\ acls03.at\ diff --git a/tests/testsuite.at b/tests/testsuite.at index db83c80..9493641 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -440,6 +440,7 @@ m4_include([xattr03.at]) m4_include([xattr04.at]) m4_include([xattr05.at]) m4_include([xattr06.at]) +m4_include([xattr07.at]) m4_include([acls01.at]) m4_include([acls02.at]) diff --git a/tests/xattr07.at b/tests/xattr07.at new file mode 100644 index 0000000..a834981 --- /dev/null +++ b/tests/xattr07.at @@ -0,0 +1,73 @@ +# Process this file with autom4te to create testsuite. -*- Autotest -*- +# +# Test suite for GNU tar. +# Copyright 2011, 2013-2014, 2016 Free Software Foundation, Inc. + +# This file is part of GNU tar. + +# GNU tar 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 of the License, or +# (at your option) any later version. + +# GNU tar 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 . +# +# Test description: +# Test that --keep-old-files doesn't change xattrs of already existing file. +# Per report: +# https://lists.gnu.org/archive/html/bug-tar/2016-10/msg00001.html + +AT_SETUP([xattrs: xattrs and --skip-old-files]) +AT_KEYWORDS([xattrs xattr07]) + +AT_TAR_CHECK([ +AT_XATTRS_PREREQ +mkdir dir +genfile --file dir/file +genfile --file dir/file2 + +setfattr -n user.test -v OurDirValue dir +setfattr -n user.test -v OurFileValue dir/file +setfattr -n user.test -v OurFileValue dir/file2 + +tar --xattrs --no-recursion -cf archive.tar dir dir/file dir/file2 + +setfattr -n user.test -v OurDirValue2 dir +setfattr -n user.test -v OurFileValue2 dir/file +setfattr -n user.test -v OurFileValue2 dir/file2 + +# Check that tar continues to file2 too! +tar --xattrs -xvf archive.tar --skip-old-files +tar --xattrs -xvf archive.tar --keep-old-files + +getfattr -h -d dir | grep -v -e '^#' -e ^$ +getfattr -h -d dir/file | grep -v -e '^#' -e ^$ +getfattr -h -d dir/file2 | grep -v -e '^#' -e ^$ +], +[0], +[dir/ +dir/file +dir/file2 +dir/ +dir/file +dir/file2 +user.test="OurDirValue2" +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 +tar: Exiting with failure status due to previous errors +]) + +AT_CLEANUP -- 2.9.3