Extend tar/test/test_option_r to verify the bug that Michihiro found

with -r brokenness.
This turns out to be a bug in the core archive_read I/O not correctly
tracking the current file position under some circumstances.
Extend libarchive/test/test_read_position to verify file position
tracking both with and without a registered skip function.
Fix the bug:  When libarchive falls back to regular reads to fill
out an internal skip request, it failed to count those
bytes toward the current file position.

SVN-Revision: 2853

diff --git a/libarchive/archive_read.c b/libarchive/archive_read.c
index 82c6c5bec5ba10e9d3657ab0c884838cda31e0d9..f15d10d2721b2a1bda40d0372370dee124801022 100644 (file)
--- a/libarchive/archive_read.c
+++ b/libarchive/archive_read.c
@@ -1284,7 +1284,6 @@ advance_file_pointer(struct archive_read_filter *filter, int64_t request)
        /* Use ordinary reads as necessary to complete the request. */
        for (;;) {
                bytes_read = (filter->read)(filter, &filter->client_buff);
-
                if (bytes_read < 0) {
                        filter->client_buff = NULL;
                        filter->fatal = 1;
@@ -1303,9 +1302,11 @@ advance_file_pointer(struct archive_read_filter *filter, int64_t request)
                        filter->client_avail = bytes_read - request;
                        filter->client_total = bytes_read;
                        total_bytes_skipped += request;
+                       filter->bytes_consumed += request;
                        return (total_bytes_skipped);
                }
 
+               filter->bytes_consumed += bytes_read;
                total_bytes_skipped += bytes_read;
                request -= bytes_read;
        }

diff --git a/libarchive/test/test_read_position.c b/libarchive/test/test_read_position.c
index cdaa783d6369472b61ed77bceb857a53565c2445..bde2ff7e82cc19f5b15931f75b5e3d943ec222e9 100644 (file)
--- a/libarchive/test/test_read_position.c
+++ b/libarchive/test/test_read_position.c
@@ -25,8 +25,42 @@
 #include "test.h"
 __FBSDID("$FreeBSD: head/lib/libarchive/test/test_read_position.c 189389 2009-03-05 02:19:42Z kientzle $");
 
-static unsigned char nulls[10000];
-static unsigned char  buff[10000000];
+static unsigned char nulls[1000];
+static unsigned char tmp[1000];
+static unsigned char  buff[10000];
+size_t data_sizes[] = {0, 5, 511, 512, 513};
+
+void
+verify_read_positions(struct archive *a)
+{
+       struct archive_entry *ae;
+       intmax_t read_position = 0;
+       size_t j;
+
+       /* Initial header position is zero. */
+       assert(read_position == (intmax_t)archive_read_header_position(a));
+       for (j = 0; j < sizeof(data_sizes)/sizeof(data_sizes[0]); ++j) {
+               assertA(0 == archive_read_next_header(a, &ae));
+               assert(read_position
+                   == (intmax_t)archive_read_header_position(a));
+               /* Every other entry: read, then skip */
+               if (j & 1)
+                       assertEqualInt(ARCHIVE_OK,
+                           archive_read_data_into_buffer(a, tmp, 1));
+               assertA(0 == archive_read_data_skip(a));
+               /* read_data_skip() doesn't change header_position */
+               assert(read_position
+                   == (intmax_t)archive_read_header_position(a));
+
+               read_position += 512; /* Size of header. */
+               read_position += (data_sizes[j] + 511) & ~511;
+       }
+
+       assertA(1 == archive_read_next_header(a, &ae));
+       assert(read_position == (intmax_t)archive_read_header_position(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a));
+       assert(read_position == (intmax_t)archive_read_header_position(a));
+}
 
 /* Check that header_position tracks correctly on read. */
 DEFINE_TEST(test_read_position)
@@ -34,9 +68,7 @@ DEFINE_TEST(test_read_position)
        struct archive *a;
        struct archive_entry *ae;
        size_t write_pos;
-       intmax_t read_position;
-       size_t i, j;
-       size_t data_sizes[] = {0, 5, 511, 512, 513};
+       size_t i;
 
        /* Sanity test */
        assert(sizeof(nulls) + 512 + 1024 <= sizeof(buff));
@@ -61,34 +93,18 @@ DEFINE_TEST(test_read_position)
        assertEqualIntA(a, ARCHIVE_OK, archive_write_close(a));
        assertEqualInt(ARCHIVE_OK, archive_write_free(a));
 
-       /* Read the archive back. */
+       /* Read the archive back with a skip function. */
        assert(NULL != (a = archive_read_new()));
        assertA(0 == archive_read_support_format_tar(a));
-       assertA(0 == archive_read_open_memory2(a, buff, sizeof(buff), 512));
-
-       read_position = 0;
-       /* Initial header position is zero. */
-       assert(read_position == (intmax_t)archive_read_header_position(a));
-       for (j = 0; j < i; ++j) {
-               assertA(0 == archive_read_next_header(a, &ae));
-               assert(read_position
-                   == (intmax_t)archive_read_header_position(a));
-               /* Every other entry: read, then skip */
-               if (j & 1)
-                       assertEqualInt(ARCHIVE_OK,
-                           archive_read_data_into_buffer(a, buff, 1));
-               assertA(0 == archive_read_data_skip(a));
-               /* read_data_skip() doesn't change header_position */
-               assert(read_position
-                   == (intmax_t)archive_read_header_position(a));
-
-               read_position += 512; /* Size of header. */
-               read_position += (data_sizes[j] + 511) & ~511;
-       }
+       assertA(0 == read_open_memory(a, buff, sizeof(buff), 512));
+       verify_read_positions(a);
+       archive_read_free(a);
 
-       assertA(1 == archive_read_next_header(a, &ae));
-       assert(read_position == (intmax_t)archive_read_header_position(a));
-       assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a));
-       assert(read_position == (intmax_t)archive_read_header_position(a));
+       /* Read the archive back without a skip function. */
+       assert(NULL != (a = archive_read_new()));
+       assertA(0 == archive_read_support_format_tar(a));
+       assertA(0 == read_open_memory2(a, buff, sizeof(buff), 512));
+       verify_read_positions(a);
        archive_read_free(a);
+
 }

diff --git a/tar/test/test_option_r.c b/tar/test/test_option_r.c
index bfbdf795a6e44cae35de409af2188fb91290241b..8d2a882af6a0ac06e936a7f95708c5eed04c1f6f 100644 (file)
--- a/tar/test/test_option_r.c
+++ b/tar/test/test_option_r.c
@@ -30,8 +30,9 @@ __FBSDID("$FreeBSD$");
  */
 DEFINE_TEST(test_option_r)
 {
-       char buff[7500];
+       char *buff;
        char *p0, *p1;
+       size_t buff_size = 35000;
        size_t s, buff_size_rounded;
        int r, i;
 
@@ -57,9 +58,11 @@ DEFINE_TEST(test_option_r)
        assertEqualMem(p0 + 1536, "\0\0\0\0\0\0\0\0", 8);
 
        /* Edit that file with a lot more data and update the archive with a new copy. */
-       for (i = 0; i < sizeof(buff); ++i)
+       buff = malloc(buff_size);
+       assert(buff != NULL);
+       for (i = 0; i < buff_size; ++i)
                buff[i] = "abcdefghijklmnopqrstuvwxyz"[rand() % 26];
-       buff[sizeof(buff) - 1] = '\0';
+       buff[buff_size - 1] = '\0';
        assertMakeFile("f1", 0644, buff);
        r = systemf("%s rf archive.tar --format=ustar f1 >step2.out 2>step2.err", testprog);
        failure("Error invoking %s rf archive.tar f1", testprog);
@@ -73,13 +76,13 @@ DEFINE_TEST(test_option_r)
                free(p0);
                return;
        }
-       buff_size_rounded = ((sizeof(buff) + 511) / 512) * 512;
+       buff_size_rounded = ((buff_size + 511) / 512) * 512;
        assert(s >= 2560 + buff_size_rounded);
        /* Verify first entry is unchanged. */
        assertEqualMem(p0, p1, 1024);
        /* Verify that second entry is correct. */
        assertEqualMem(p1 + 1024, "f1", 3);
-       assertEqualMem(p1 + 1536, buff, sizeof(buff));
+       assertEqualMem(p1 + 1536, buff, buff_size);
        /* Verify end-of-archive marker. */
        assertEqualMem(p1 + 1536 + buff_size_rounded, "\0\0\0\0\0\0\0\0", 8);
        assertEqualMem(p1 + 2048 + buff_size_rounded, "\0\0\0\0\0\0\0\0", 8);