[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-tar] --remove-files deletes files even if archive couldn't be c
|
From: |
Sergey Poznyakoff |
|
Subject: |
Re: [Bug-tar] --remove-files deletes files even if archive couldn't be created, when used with compression |
|
Date: |
Wed, 07 Oct 2009 16:52:47 +0300 |
Hi Bart,
> the point is that tar should actually 'exit now' when it says 'exiting
> now' instead of proceeding to delete the files without putting them in
> a usable archive.
It is fixed in the repository. The patch is attached.
Regards,
Sergey
>From 4dfcd6c054a5e9e1a371c822a3be90564dd9b690 Mon Sep 17 00:00:00 2001
From: Sergey Poznyakoff <address@hidden>
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 <obstack.h>
+#include <progname.h>
#include <paxlib.h>
@@ -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 <system.h>
+#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 <system.h>
+#include "common.h"
+#include <quotearg.h>
+
+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: <address@hidden>
+# 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: <address@hidden>
+# 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])
--
1.6.4.2