Fix a bug in multi-volume archive creation.

When creating multivolume archives, the bufmap code in buffer.c
implicitly assumed that the members are stored in the archive
contiguously, ignoring the member (and eventual extended) headers
between them.  This worked until the member header happened to be
at the very beginning of the volume, in which case its length was
included in the calculation of the stored size and size left to
store.  Due to this, the GNUFileParts extended header contained
invalid GNU.volume.offset value, and the resulting archive failed
to extract properly.

This patch also eliminates improper listing of file part headers
as regular files, when creating multivolume posix archives with -v.

* src/buffer.c (bufmap): New member nblocks.  Counts number of blocks
of file data written since reset.
(bufmap_reset): Reset nblocks to 0.
(_flush_write): Update nblocks.  When computing offset difference for
bufmap_reset, count only data blocks, not headers.
(close_archive): Flush archive until all blocks are written.
(add_chunk_header): Use simple_finish_header instead of finish_header
to avoid listing chunk header as regular file in verbose mode.
* tests/multiv10.at: New test case.
* tests/Makefile.am: Add new test.
* tests/testsuite.at: Add new test.

diff --git a/src/buffer.c b/src/buffer.c
index fb34834280b14f0e94e3a6da1edc13c2f185dc8a..57fe813ada809f65af3d62c3941a7c5d789ff26e 100644 (file)
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -105,13 +105,20 @@ bool write_archive_to_stdout;
 
 /* When creating a multi-volume archive, each 'bufmap' represents
    a member stored (perhaps partly) in the current record buffer.
+   Bufmaps are form a single-linked list in chronological order.
+   
    After flushing the record to the output media, all bufmaps that
-   represent fully written members are removed from the list, then
-   the sizeleft and start numbers in the remaining bufmaps are updated.
+   represent fully written members are removed from the list, the
+   nblocks and sizeleft values in the bufmap_head and start values
+   in all remaining bufmaps are updated.  The information stored
+   in bufmap_head is used to form the volume header.
 
    When reading from a multi-volume archive, the list degrades to a
    single element, which keeps information about the member currently
-   being read.
+   being read.  In that case the sizeleft member is updated explicitly
+   from the extractor code by calling the mv_size_left function.  The
+   information from bufmap_head is compared with the volume header data
+   to ensure that subsequent volumes are fed in the right order.
 */
 
 struct bufmap
@@ -121,6 +128,7 @@ struct bufmap
   char *file_name;              /* Name of the stored file */
   off_t sizetotal;              /* Size of the stored file */
   off_t sizeleft;               /* Size left to read/write */
+  size_t nblocks;               /* Number of blocks written since reset */
 };
 static struct bufmap *bufmap_head, *bufmap_tail;
 
@@ -145,6 +153,7 @@ mv_begin_write (const char *file_name, off_t totsize, off_t sizeleft)
       bp->file_name = xstrdup (file_name);
       bp->sizetotal = totsize;
       bp->sizeleft = sizeleft;
+      bp->nblocks = 0;
     }
 }
 
@@ -155,8 +164,7 @@ bufmap_locate (size_t off)
 
   for (map = bufmap_head; map; map = map->next)
     {
-      if (!map->next
-         || off < map->next->start * BLOCKSIZE)
+      if (!map->next || off < map->next->start * BLOCKSIZE)
        break;
     }
   return map;
@@ -185,7 +193,10 @@ bufmap_reset (struct bufmap *map, ssize_t fixup)
   if (map)
     {
       for (; map; map = map->next)
-       map->start += fixup;
+       {
+         map->start += fixup;
+         map->nblocks = 0;
+       }
     }
 }
 
@@ -868,12 +879,19 @@ _flush_write (void)
       if (map)
        {
          size_t delta = status - map->start * BLOCKSIZE;
+         ssize_t diff;
+         map->nblocks += delta / BLOCKSIZE;
          if (delta > map->sizeleft)
            delta = map->sizeleft;
          map->sizeleft -= delta;
          if (map->sizeleft == 0)
-           map = map->next;
-         bufmap_reset (map, map ? (- map->start) : 0);
+           {
+             diff = map->start + map->nblocks;
+             map = map->next;
+           }
+         else
+           diff = map->start;
+         bufmap_reset (map, - diff);
        }
     }
   return status;
@@ -1105,9 +1123,9 @@ close_archive (void)
 {
   if (time_to_start_writing || access_mode == ACCESS_WRITE)
     {
-      flush_archive ();
-      if (current_block > record_start)
-        flush_archive ();
+      do
+       flush_archive ();
+      while (current_block > record_start);
     }
 
   compute_duration ();
@@ -1711,7 +1729,6 @@ add_chunk_header (struct bufmap *map)
 {
   if (archive_format == POSIX_FORMAT)
     {
-      off_t block_ordinal;
       union block *blk;
       struct tar_stat_info st;
 
@@ -1726,11 +1743,10 @@ add_chunk_header (struct bufmap *map)
       st.file_name = st.orig_file_name;
       st.archive_file_size = st.stat.st_size = map->sizeleft;
 
-      block_ordinal = current_block_ordinal ();
       blk = start_header (&st);
       if (!blk)
         abort (); /* FIXME */
-      finish_header (&st, blk, block_ordinal);
+      simple_finish_header (write_extended (false, &st, blk));
       free (st.orig_file_name);
     }
 }

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 92516fb39e1bd8297a48ff7bb6400a57272f92c9..70711676fffc889d312466b234b537dba34ba829 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -155,6 +155,7 @@ TESTSUITE_AT = \
  multiv07.at\
  multiv08.at\
  multiv09.at\
+ multiv10.at\
  numeric.at\
  old.at\
  onetop01.at\

diff --git a/tests/multiv10.at b/tests/multiv10.at
new file mode 100644 (file)
index 0000000..03fbf15
--- /dev/null
+++ b/tests/multiv10.at
@@ -0,0 +1,59 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+# Test suite for GNU tar.
+# Copyright 2015-2017 Free Software Foundation, Inc.
+#
+# 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 <http://www.gnu.org/licenses/>.
+
+# Description: When creating multivolume archives, the bufmap code in
+# buffer.c implicitly assumed that the members are stored in the archive
+# in a contiguous fashion, ignoring the member (and eventual extended) headers
+# between them.  This worked until the member header happened to be at
+# the very beginning of the volume, in which case its length was included in
+# the calculation of the stored size and size left to store.  Due to this,
+# the GNUFileParts extended header contained invalid GNU.volume.offset value,
+# and the resulting archive failed to extract properly.  The bug affected
+# versions of tar up to 1.29.90 (commit da8d0659a6).
+#
+# This test case also checks that GNUFileParts headers are not displayed in
+# verbose mode.
+# 
+# Reported by: <russiangolem@gmail.com>
+# References:
+#     <CAE7Kiz_0oMqGdzkoh0FbOd=hUoPhtHHYhjZveM_4hEku081QFQ@mail.gmail.com>,
+#     http://lists.gnu.org/archive/html/bug-tar/2017-05/msg00007.html
+#
+
+AT_SETUP([file start at the beginning of a posix volume])
+AT_KEYWORDS([multivolume multiv multiv10])
+
+AT_TAR_CHECK([
+set -e
+genfile --length=15360 --file data1
+genfile --length=15360 --file data2
+tar -v -c -L 10 -M -f 1.tar -f 2.tar -f 3.tar -f 4.tar -f 5.tar data1 data2
+tar -M -t -f 1.tar -f 2.tar -f 3.tar -f 4.tar -f 5.tar
+mkdir out
+tar -C out -M -x -f 1.tar -f 2.tar -f 3.tar -f 4.tar -f 5.tar
+cmp data1 out/data1
+cmp data2 out/data2
+],
+[0],
+[data1
+data2
+data1
+data2
+],
+[],[],[],[posix])
+
+AT_CLEANUP

diff --git a/tests/testsuite.at b/tests/testsuite.at
index 483a7ea0697c33e17dccf38e520e3752326ab511..8c97e34c04eff8c7b39c6d1126049d1ec2902b66 100644 (file)
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -377,6 +377,7 @@ m4_include([multiv06.at])
 m4_include([multiv07.at])
 m4_include([multiv08.at])
 m4_include([multiv09.at])
+m4_include([multiv10.at])
 
 AT_BANNER([Owner and Groups])
 m4_include([owner.at])