From: Tim Kientzle Date: Sun, 2 Jan 2011 20:03:04 +0000 (-0500) Subject: Extend tar/test/test_option_r to verify the bug that Michihiro found X-Git-Tag: v3.0.0a~793 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=055f150d0de491c646906f575438ad492f134252;p=thirdparty%2Flibarchive.git 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 82c6c5bec..f15d10d27 100644 --- 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 cdaa783d6..bde2ff7e8 100644 --- 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 bfbdf795a..8d2a882af 100644 --- 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);