Issue #655: Be more careful verifying sparse file reading

The old logic took a number of shortcuts; this now does a careful
comparison of the template used to generate the file with
the contents read back, verifying that the read blocks reflect
the expected pattern of zero and non-zero bytes and that the
expected number of holes actually appear.

diff --git a/libarchive/test/main.c b/libarchive/test/main.c
index b39c2bf78d70508e07c4ac39d479e793d12ec535..88d72b311f9fb9484dccfe852926e4e63c0ac607 100644 (file)
--- a/libarchive/test/main.c
+++ b/libarchive/test/main.c
@@ -779,6 +779,33 @@ assertion_equal_mem(const char *file, int line,
        return (0);
 }
 
+/* Verify that a block of memory is filled with the specified byte. */
+int
+assertion_memory_filled_with(const char *file, int line,
+    const void *_v1, const char *vd,
+    size_t l, const char *ld,
+    char b, const char *bd, void *extra)
+{
+       const char *v1 = (const char *)_v1;
+       size_t c = 0;
+       (void)ld; /* UNUSED */
+
+       assertion_count(file, line);
+
+       for (size_t i = 0; i < l; ++i) {
+               if (v1[i] == b) {
+                       ++c;
+               }
+       }
+       if (c == l)
+               return (1);
+
+       failure_start(file, line, "%s (size %d) not filled with %s", vd, (int)l, bd);
+       logprintf("   Only %d bytes were correct\n", (int)c);
+       failure_finish(extra);
+       return (0);
+}
+
 /* Verify that the named file exists and is empty. */
 int
 assertion_empty_file(const char *filename, int line, const char *f1)

diff --git a/libarchive/test/test.h b/libarchive/test/test.h
index ad3de27ba7693081b057bb11b08a4d35d51962e3..1117d6a77760324ee41a958b19e68f14268fb561 100644 (file)
--- a/libarchive/test/test.h
+++ b/libarchive/test/test.h
@@ -145,6 +145,9 @@
 /* As above, but raw blocks of bytes. */
 #define assertEqualMem(v1, v2, l)      \
   assertion_equal_mem(__FILE__, __LINE__, (v1), #v1, (v2), #v2, (l), #l, NULL)
+/* Assert that memory is full of a specified byte */
+#define assertMemoryFilledWith(v1, l, b)                                       \
+  assertion_memory_filled_with(__FILE__, __LINE__, (v1), #v1, (l), #l, (b), #b, NULL)
 /* Assert two files are the same. */
 #define assertEqualFile(f1, f2)        \
   assertion_equal_file(__FILE__, __LINE__, (f1), (f2))
@@ -228,6 +231,7 @@ int assertion_empty_file(const char *, int, const char *);
 int assertion_equal_file(const char *, int, const char *, const char *);
 int assertion_equal_int(const char *, int, long long, const char *, long long, const char *, void *);
 int assertion_equal_mem(const char *, int, const void *, const char *, const void *, const char *, size_t, const char *, void *);
+int assertion_memory_filled_with(const char *, int, const void *, const char *, size_t, const char *, char, const char *, void *);
 int assertion_equal_string(const char *, int, const char *v1, const char *, const char *v2, const char *, void *, int);
 int assertion_equal_wstring(const char *, int, const wchar_t *v1, const char *, const wchar_t *v2, const char *, void *);
 int assertion_file_atime(const char *, int, const char *, long, long);

diff --git a/libarchive/test/test_sparse_basic.c b/libarchive/test/test_sparse_basic.c
index 09c0c9f5e8dc26b76b1b99cdb23eacb5f5582db9..f72594bb7f3a56a35ce1ebec26d1bfb8bfd159ee 100644 (file)
--- a/libarchive/test/test_sparse_basic.c
+++ b/libarchive/test/test_sparse_basic.c
@@ -50,6 +50,11 @@ __FBSDID("$FreeBSD$");
 #include <linux/fs.h>
 #endif
 
+/* The logic to compare sparse file data read from disk with the
+ * specification is a little involved.  Set to 1 to have the progress
+ * dumped. */
+#define DEBUG 0
+
 /*
  * NOTE: On FreeBSD and Solaris, this test needs ZFS.
  * You may should perfom this test as
@@ -259,39 +264,114 @@ create_sparse_file(const char *path, const struct sparse *s)
  */
 static void
 verify_sparse_file(struct archive *a, const char *path,
-    const struct sparse *sparse, int blocks, int expected_data_blocks)
+    const struct sparse *sparse, int expected_holes)
 {
        struct archive_entry *ae;
        const void *buff;
        size_t bytes_read;
-       int64_t offset;
-       int64_t total;
-       int data_blocks, hole;
+       int64_t offset, expected_offset, last_offset;
+       int holes_seen = 0;
 
        create_sparse_file(path, sparse);
        assert((ae = archive_entry_new()) != NULL);
        assertEqualIntA(a, ARCHIVE_OK, archive_read_disk_open(a, path));
        assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header2(a, ae));
-       /* Verify the number of holes only, not its offset nor its
-        * length because those alignments are deeply dependence on
-        * its filesystem. */ 
-       assertEqualInt(blocks, archive_entry_sparse_count(ae));
-       total = 0;
-       data_blocks = 0;
-       hole = 0;
+
+       expected_offset = 0;
+       last_offset = 0;
        while (ARCHIVE_OK == archive_read_data_block(a, &buff, &bytes_read,
            &offset)) {
-               if (offset > total || offset == 0) {
-                       if (offset > total)
-                               hole = 1;
-                       data_blocks++;
+#if DEBUG
+               fprintf(stderr, "%s: bytes_read=%d offset=%d\n", path, (int)bytes_read, (int)offset);
+#endif
+               if (offset > last_offset) {
+                       ++holes_seen;
+               }
+               /* Blocks entirely before the data we just read. */
+               while (expected_offset + (int64_t)sparse->size < offset) {
+#if DEBUG
+                       fprintf(stderr, "    skipping expected_offset=%d, size=%d\n", (int)expected_offset, (int)sparse->size);
+#endif
+                       /* Must be holes. */
+                       assert(sparse->type == HOLE);
+                       expected_offset += sparse->size;
+                       ++sparse;
                }
-               total = offset + bytes_read;
+               /* Block that overlaps beginning of data */
+               const char *start = buff;
+               if (expected_offset < offset
+                   && expected_offset + (int64_t)sparse->size <= offset + (int64_t)bytes_read) {
+                       const char *end = buff + (expected_offset - offset) + (size_t)sparse->size;
+#if DEBUG
+                       fprintf(stderr, "    overlapping hole expected_offset=%d, size=%d\n", (int)expected_offset, (int)sparse->size);
+#endif
+                       /* Must be a hole, overlap must be filled with '\0' */
+                       if (assert(sparse->type == HOLE)) {
+                               assertMemoryFilledWith(start, end - start, '\0');
+                       }
+                       start = end;
+                       expected_offset += sparse->size;
+                       ++sparse;
+               }
+               /* Blocks completely contained in data we just read. */
+               while (expected_offset + (int64_t)sparse->size <= offset + (int64_t)bytes_read) {
+                       const char *end = buff + (expected_offset - offset) + (size_t)sparse->size;
+                       if (sparse->type == HOLE) {
+#if DEBUG
+                               fprintf(stderr, "    contained hole expected_offset=%d, size=%d\n", (int)expected_offset, (int)sparse->size);
+#endif
+
+                               /* verify data corresponding to hole is '\0' */
+                               if (end > (const char *)buff + bytes_read) {
+                                       end = buff + bytes_read;
+                               }
+                               assertMemoryFilledWith(start, end - start, '\0');
+                               start = end;
+                               expected_offset += sparse->size;
+                               ++sparse;
+                       } else if (sparse->type == DATA) {
+#if DEBUG
+                               fprintf(stderr, "    contained data expected_offset=%d, size=%d\n", (int)expected_offset, (int)sparse->size);
+#endif
+                               /* verify data corresponding to hole is ' ' */
+                               if (assert(expected_offset + sparse->size <= offset + bytes_read)) {
+                                       assert(start == buff + (size_t)(expected_offset - offset));
+                                       assertMemoryFilledWith(start, end - start, ' ');
+                               }
+                               start = end;
+                               expected_offset += sparse->size;
+                               ++sparse;
+                       } else {
+                               break;
+                       }
+               }
+               /* Block that overlaps end of data */
+               if (expected_offset < offset + (int64_t)bytes_read) {
+                       const char *end = buff + bytes_read;
+#if DEBUG
+                       fprintf(stderr, "    trailing overlap expected_offset=%d, size=%d\n", (int)expected_offset, (int)sparse->size);
+#endif
+                       /* Must be a hole, overlap must be filled with '\0' */
+                       if (assert(sparse->type == HOLE)) {
+                               assertMemoryFilledWith(start, end - start, '\0');
+                       }
+               }
+               last_offset = offset + bytes_read;
        }
-       assertEqualInt(expected_data_blocks, data_blocks);
+       /* Count a hole at EOF? */
+       if (last_offset < archive_entry_size(ae)) {
+               ++holes_seen;
+       }
+
+       /* Verify blocks after last read */
+       while (sparse->type == HOLE) {
+               expected_offset += sparse->size;
+               ++sparse;
+       }
+       assert(sparse->type == END);
+       assertEqualInt(expected_offset, archive_entry_size(ae));
 
-       if (blocks > 1)
-               assert(hole); /* There must be a hole if > 1 blocks were encoded */
+       assertEqualInt(holes_seen, expected_holes);
 
        assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a));
        archive_entry_free(ae);
@@ -402,9 +482,10 @@ DEFINE_TEST(test_sparse_basic)
                { END,  0 }
        };
        const struct sparse sparse_file3[] = {
-               /* This hole size is too small to create a sparse
-                * files for almost filesystem. */
-               { HOLE,  1024 }, { DATA, 10240 },
+               /* This hole size is too small to create a sparse file */
+               { HOLE,  1 }, { DATA, 10240 },
+               { HOLE,  1 }, { DATA, 10240 },
+               { HOLE,  1 }, { DATA, 10240 },
                { END,  0 }
        };
 
@@ -436,11 +517,11 @@ DEFINE_TEST(test_sparse_basic)
         */
        assert((a = archive_read_disk_new()) != NULL);
 
-       verify_sparse_file(a, "file0", sparse_file0, 5, 5);
-       verify_sparse_file(a, "file1", sparse_file1, 2, 2);
-       verify_sparse_file(a, "file2", sparse_file2, 20, 20);
+       verify_sparse_file(a, "file0", sparse_file0, 4);
+       verify_sparse_file(a, "file1", sparse_file1, 3);
+       verify_sparse_file(a, "file2", sparse_file2, 20);
        /* Encoded non sparse; expect a data block but no sparse entries. */
-       verify_sparse_file(a, "file3", sparse_file3, 0, 1);
+       verify_sparse_file(a, "file3", sparse_file3, 0);
 
        assertEqualInt(ARCHIVE_OK, archive_read_free(a));
 
@@ -484,7 +565,7 @@ DEFINE_TEST(test_fully_sparse_files)
        assert((a = archive_read_disk_new()) != NULL);
 
        /* Fully sparse files are encoded with a zero-length "data" block. */
-       verify_sparse_file(a, "file0", sparse_file, 1, 1);
+       verify_sparse_file(a, "file0", sparse_file, 1);
 
        assertEqualInt(ARCHIVE_OK, archive_read_free(a));
        free(cwd);