From 4dfcd6c054a5e9e1a371c822a3be90564dd9b690 Mon Sep 17 00:00:00 2001 From: Sergey Poznyakoff Date: Wed, 7 Oct 2009 16:42:06 +0300 Subject: [PATCH] Fix bugs in handling the --remove-files option. Make sure the files are deleted only if they were succesfully stored to the archive. * src/exit.c: New file. * src/unlink.c: New file. * src/Makefile.am (tar_SOURCES): Add exit.c and unlink.c. * src/common.h: Include progname.h (program_name): Remove global. (records_written): New extern. (queue_deferred_unlink, finish_deferred_unlinks): New prototypes. (fatal_exit_hook): New extern. * src/create.c (create_archive): Call finish_deferred_unlinks. (dump_hard_link, dump_file0): Don't actually unlink the file, queue it to deferred_unlinks instead. * src/delete.c (records_written): Remove extern: declared in common.h. * src/extract.c (extract_archive): Set fatal_exit_hook. (fatal_exit, xalloc_die): Move to exit.c * src/system.c (sys_wait_for_child): Exit immediately if the child dies or exits with a non-zero status. (sys_child_open_for_compress) (sys_child_open_for_uncompress): Use set_program_name, instead of setting program_name directly. * src/tar.c (main): Use set_program_name, instead of setting program_name directly. * tests/Makefile.am (TESTSUITE_AT): Add remfiles01.at and remfiles02.at. * tests/testsuite.at: Likewise. * tests/gzip.at: Reflect the above changes. --- src/Makefile.am | 2 + src/common.h | 13 +++- src/create.c | 30 ++------- src/delete.c | 1 - src/exit.c | 37 +++++++++++ src/extract.c | 17 +---- src/system.c | 14 ++-- src/tar.c | 2 +- src/unlink.c | 158 ++++++++++++++++++++++++++++++++++++++++++++ tests/Makefile.am | 2 + tests/gzip.at | 7 +- tests/remfiles01.at | 57 ++++++++++++++++ tests/remfiles02.at | 58 ++++++++++++++++ tests/testsuite.at | 3 + 14 files changed, 347 insertions(+), 54 deletions(-) create mode 100644 src/exit.c create mode 100644 src/unlink.c create mode 100644 tests/remfiles01.at create mode 100644 tests/remfiles02.at diff --git a/src/Makefile.am b/src/Makefile.am index 9f268c3..fcbac33 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -27,6 +27,7 @@ tar_SOURCES = \ compare.c\ create.c\ delete.c\ + exit.c\ extract.c\ xheader.c\ incremen.c\ @@ -38,6 +39,7 @@ tar_SOURCES = \ system.c\ tar.c\ transform.c\ + unlink.c\ update.c\ utf8.c\ warning.c diff --git a/src/common.h b/src/common.h index 1a6ca8b..c2a92d2 100644 --- a/src/common.h +++ b/src/common.h @@ -60,6 +60,7 @@ #define obstack_chunk_alloc xmalloc #define obstack_chunk_free free #include +#include #include @@ -70,9 +71,6 @@ /* Information gleaned from the command line. */ -/* Name of this program. */ -GLOBAL const char *program_name; - /* Main command option. */ enum subcommand @@ -400,6 +398,7 @@ extern char *volume_label; extern char *continued_file_name; extern uintmax_t continued_file_size; extern uintmax_t continued_file_offset; +extern off_t records_written; size_t available_space_after (union block *pointer); off_t current_block_ordinal (void); @@ -827,3 +826,11 @@ extern int warning_option; } \ while (0) +/* Module unlink.c */ + +void queue_deferred_unlink (const char *name, bool is_dir); +void finish_deferred_unlinks (void); + +/* Module exit.c */ +extern void (*fatal_exit_hook) (void); + diff --git a/src/create.c b/src/create.c index 79c80ce..d4b9ae7 100644 --- a/src/create.c +++ b/src/create.c @@ -1333,7 +1333,7 @@ create_archive (void) write_eot (); close_archive (); - + finish_deferred_unlinks (); if (listed_incremental_option) write_directory_file (); } @@ -1413,8 +1413,8 @@ dump_hard_link (struct tar_stat_info *st) blk->header.typeflag = LNKTYPE; finish_header (st, blk, block_ordinal); - if (remove_files_option && unlink (st->orig_file_name) != 0) - unlink_error (st->orig_file_name); + if (remove_files_option) + queue_deferred_unlink (st->orig_file_name, false); return true; } @@ -1680,18 +1680,7 @@ dump_file0 (struct tar_stat_info *st, const char *p, } if (ok && remove_files_option) - { - if (is_dir) - { - if (rmdir (p) != 0 && errno != ENOTEMPTY) - rmdir_error (p); - } - else - { - if (unlink (p) != 0) - unlink_error (p); - } - } + queue_deferred_unlink (p, is_dir); return; } @@ -1727,10 +1716,8 @@ dump_file0 (struct tar_stat_info *st, const char *p, /* nothing more to do to it */ if (remove_files_option) - { - if (unlink (p) == -1) - unlink_error (p); - } + queue_deferred_unlink (p, false); + file_count_links (st); return; } @@ -1782,10 +1769,7 @@ dump_file0 (struct tar_stat_info *st, const char *p, finish_header (st, header, block_ordinal); if (remove_files_option) - { - if (unlink (p) == -1) - unlink_error (p); - } + queue_deferred_unlink (p, false); } void diff --git a/src/delete.c b/src/delete.c index d59a857..a67993c 100644 --- a/src/delete.c +++ b/src/delete.c @@ -35,7 +35,6 @@ extern union block *current_block; extern union block *recent_long_name; extern union block *recent_long_link; extern off_t records_read; -extern off_t records_written; /* The number of records skipped at the start of the archive, when passing over members that are not deleted. */ diff --git a/src/exit.c b/src/exit.c new file mode 100644 index 0000000..ad4d27c --- /dev/null +++ b/src/exit.c @@ -0,0 +1,37 @@ +/* This file is part of GNU tar. + Copyright (C) 2009 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, write to the Free Software Foundation, Inc., + 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ + +#include +#include "common.h" + +void (*fatal_exit_hook) (void); + +void +fatal_exit (void) +{ + if (fatal_exit_hook) + fatal_exit_hook (); + error (TAREXIT_FAILURE, 0, _("Error is not recoverable: exiting now")); + abort (); +} + +void +xalloc_die (void) +{ + error (0, 0, "%s", _("memory exhausted")); + fatal_exit (); +} diff --git a/src/extract.c b/src/extract.c index 3c92e53..5f12cf9 100644 --- a/src/extract.c +++ b/src/extract.c @@ -1242,6 +1242,8 @@ extract_archive (void) char typeflag; tar_extractor_t fun; + fatal_exit_hook = extract_finish; + /* Try to disable the ability to unlink a directory. */ priv_set_remove_linkdir (); @@ -1406,18 +1408,3 @@ rename_directory (char *src, char *dst) } return true; } - -void -fatal_exit (void) -{ - extract_finish (); - error (TAREXIT_FAILURE, 0, _("Error is not recoverable: exiting now")); - abort (); -} - -void -xalloc_die (void) -{ - error (0, 0, "%s", _("memory exhausted")); - fatal_exit (); -} diff --git a/src/system.c b/src/system.c index 7df8122..cf39dbd 100644 --- a/src/system.c +++ b/src/system.c @@ -174,11 +174,11 @@ sys_wait_for_child (pid_t child_pid, bool eof) { int sig = WTERMSIG (wait_status); if (!(!eof && sig == SIGPIPE)) - ERROR ((0, 0, _("Child died with signal %d"), sig)); + FATAL_ERROR ((0, 0, _("Child died with signal %d"), sig)); } else if (WEXITSTATUS (wait_status) != 0) - ERROR ((0, 0, _("Child returned status %d"), - WEXITSTATUS (wait_status))); + FATAL_ERROR ((0, 0, _("Child returned status %d"), + WEXITSTATUS (wait_status))); } } @@ -330,7 +330,7 @@ sys_child_open_for_compress (void) /* The new born child tar is here! */ - program_name = _("tar (child)"); + set_program_name (_("tar (child)")); xdup2 (parent_pipe[PREAD], STDIN_FILENO); xclose (parent_pipe[PWRITE]); @@ -374,7 +374,7 @@ sys_child_open_for_compress (void) { /* The newborn grandchild tar is here! Launch the compressor. */ - program_name = _("tar (grandchild)"); + set_program_name (_("tar (grandchild)")); xdup2 (child_pipe[PWRITE], STDOUT_FILENO); xclose (child_pipe[PREAD]); @@ -473,7 +473,7 @@ sys_child_open_for_uncompress (void) /* The newborn child tar is here! */ - program_name = _("tar (child)"); + set_program_name (_("tar (child)")); xdup2 (parent_pipe[PWRITE], STDOUT_FILENO); xclose (parent_pipe[PREAD]); @@ -508,7 +508,7 @@ sys_child_open_for_uncompress (void) { /* The newborn grandchild tar is here! Launch the uncompressor. */ - program_name = _("tar (grandchild)"); + set_program_name (_("tar (grandchild)")); xdup2 (child_pipe[PREAD], STDIN_FILENO); xclose (child_pipe[PWRITE]); diff --git a/src/tar.c b/src/tar.c index e3300fb..0c59352 100644 --- a/src/tar.c +++ b/src/tar.c @@ -2452,7 +2452,7 @@ int main (int argc, char **argv) { set_start_time (); - program_name = argv[0]; + set_program_name (argv[0]); setlocale (LC_ALL, ""); bindtextdomain (PACKAGE, LOCALEDIR); diff --git a/src/unlink.c b/src/unlink.c new file mode 100644 index 0000000..2af6f99 --- /dev/null +++ b/src/unlink.c @@ -0,0 +1,158 @@ +/* This file is part of GNU tar. + Copyright (C) 2009 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, write to the Free Software Foundation, Inc., + 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ + +#include +#include "common.h" +#include + +struct deferred_unlink + { + struct deferred_unlink *next; /* Next unlink in the queue */ + char *file_name; /* Absolute name of the file to unlink */ + bool is_dir; /* True if file_name is a directory */ + off_t records_written; /* Number of records written when this + entry got added to the queue */ + }; + +/* The unlink queue */ +static struct deferred_unlink *dunlink_head, *dunlink_tail; + +/* Number of entries in the queue */ +static size_t dunlink_count; + +/* List of entries available for allocation */ +static struct deferred_unlink *dunlink_avail; + +/* Delay (number of records written) between adding entry to the + list and its actual removal. */ +size_t deferred_unlink_delay = 0; + +static struct deferred_unlink * +dunlink_alloc () +{ + struct deferred_unlink *p; + if (dunlink_avail) + { + p = dunlink_avail; + dunlink_avail = p->next; + p->next = NULL; + } + else + p = xmalloc (sizeof (*p)); + return p; +} + +static void +dunlink_reclaim (struct deferred_unlink *p) +{ + free (p->file_name); + p->next = dunlink_avail; + dunlink_avail = p; +} + +static void +flush_deferred_unlinks (bool force) +{ + struct deferred_unlink *p, *prev = NULL; + + for (p = dunlink_head; p; ) + { + struct deferred_unlink *next = p->next; + if (force + || records_written > p->records_written + deferred_unlink_delay) + { + if (p->is_dir) + { + if (rmdir (p->file_name) != 0) + { + switch (errno) + { + case ENOENT: + /* nothing to worry about */ + break; + case ENOTEMPTY: + if (!force) + { + /* Keep the record in list, in the hope we'll + be able to remove it later */ + prev = p; + p = next; + continue; + } + /* fall through */ + default: + rmdir_error (p->file_name); + } + } + } + else + { + if (unlink (p->file_name) != 0 && errno != ENOENT) + unlink_error (p->file_name); + } + dunlink_reclaim (p); + dunlink_count--; + p = next; + if (prev) + prev->next = p; + else + dunlink_head = p; + } + else + { + prev = p; + p = next; + } + } + if (!dunlink_head) + dunlink_tail = NULL; +} + +void +finish_deferred_unlinks () +{ + flush_deferred_unlinks (true); + while (dunlink_avail) + { + struct deferred_unlink *next = dunlink_avail->next; + free (dunlink_avail); + dunlink_avail = next; + } +} + +void +queue_deferred_unlink (const char *name, bool is_dir) +{ + struct deferred_unlink *p; + + if (dunlink_head + && records_written > dunlink_head->records_written + deferred_unlink_delay) + flush_deferred_unlinks (false); + + p = dunlink_alloc (); + p->next = NULL; + p->file_name = normalize_filename (name); + p->is_dir = is_dir; + p->records_written = records_written; + + if (dunlink_tail) + dunlink_tail->next = p; + else + dunlink_head = p; + dunlink_tail = p; + dunlink_count++; +} diff --git a/tests/Makefile.am b/tests/Makefile.am index 51ad526..787b9d0 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -113,6 +113,8 @@ TESTSUITE_AT = \ rename03.at\ rename04.at\ rename05.at\ + remfiles01.at\ + remfiles02.at\ same-order01.at\ same-order02.at\ shortfile.at\ diff --git a/tests/gzip.at b/tests/gzip.at index eb43030..38d3538 100644 --- a/tests/gzip.at +++ b/tests/gzip.at @@ -1,7 +1,7 @@ # Process this file with autom4te to create testsuite. -*- Autotest -*- # Test suite for GNU tar. -# Copyright (C) 2004, 2007 Free Software Foundation, Inc. +# Copyright (C) 2004, 2007, 2009 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 @@ -28,14 +28,13 @@ unset TAR_OPTIONS AT_CHECK([ AT_GZIP_PREREQ tar xfvz /dev/null -test $? = 2 || exit 1 ], -[0], +[2], [], [ gzip: stdin: unexpected end of file tar: Child returned status 1 -tar: Exiting with failure status due to previous errors +tar: Error is not recoverable: exiting now ], [],[]) diff --git a/tests/remfiles01.at b/tests/remfiles01.at new file mode 100644 index 0000000..8a668a7 --- /dev/null +++ b/tests/remfiles01.at @@ -0,0 +1,57 @@ +# Process this file with autom4te to create testsuite. -*- Autotest -*- + +# Test suite for GNU tar. +# Copyright (C) 2009 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, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. + +# Description: When called with --create --remove-files and a compression +# options tar (v. <= 1.22.90) would remove files even if it had failed +# to store them in the archive. +# +# References: <77cb99c00910020940k6ce15da4wb564d2418ec52cfb@mail.gmail.com> +# http://lists.gnu.org/archive/html/bug-tar/2009-10/msg00005.html + +AT_SETUP([remove-files with compression]) +AT_KEYWORDS([create remove-files remfiles01 gzip]) + +unset TAR_OPTIONS +AT_CHECK([ +AT_GZIP_PREREQ +AT_SORT_PREREQ + +mkdir dir +cd dir +genfile --file a --length 0 +chmod 0 a +genfile --file b +mkdir c + +tar -c -f a -z --remove-files b c + +find . | sort +], +[0], +[. +./a +./b +./c +], +[tar (child): a: Cannot open: Permission denied +tar (child): Error is not recoverable: exiting now +]) + +AT_CLEANUP diff --git a/tests/remfiles02.at b/tests/remfiles02.at new file mode 100644 index 0000000..d137433 --- /dev/null +++ b/tests/remfiles02.at @@ -0,0 +1,58 @@ +# Process this file with autom4te to create testsuite. -*- Autotest -*- + +# Test suite for GNU tar. +# Copyright (C) 2009 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, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. + +# Description: When called with --create --remove-files and a compression +# options tar (v. <= 1.22.90) would remove files even if it had failed +# to store them in the archive. +# +# References: <77cb99c00910020940k6ce15da4wb564d2418ec52cfb@mail.gmail.com> +# http://lists.gnu.org/archive/html/bug-tar/2009-10/msg00005.html + +AT_SETUP([remove-files with compression: grand-child]) +AT_KEYWORDS([create remove-files remfiles02 gzip]) + +unset TAR_OPTIONS +AT_CHECK([ +AT_GZIP_PREREQ +AT_SORT_PREREQ + +mkdir dir +cd dir +mkdir a +genfile --file b +mkdir c + +tar -c -f a -z --remove-files b c + +find . | sort +], +[0], +[. +./a +./b +./c +], +[tar (child): a: Cannot open: Is a directory +tar (child): Error is not recoverable: exiting now +tar: Child returned status 2 +tar: Error is not recoverable: exiting now +]) + +AT_CLEANUP diff --git a/tests/testsuite.at b/tests/testsuite.at index 325f9d0..17bec7e 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -218,6 +218,9 @@ m4_include([shortupd.at]) m4_include([truncate.at]) m4_include([grow.at]) +m4_include([remfiles01.at]) +m4_include([remfiles02.at]) + m4_include([star/gtarfail.at]) m4_include([star/gtarfail2.at]) -- 2.44.0