From FreeBSD r196961: Updates to iso9660 tests and improved comments

explaining the iso9660 hardlink handling.

SVN-Revision: 1442

diff --git a/libarchive/archive_read_support_format_iso9660.c b/libarchive/archive_read_support_format_iso9660.c
index 287e15d82392d9c4a0bcf3916f354d68cd5bf9f0..b9a5f4dc6eb3447d50e872550e3dca0f8e283109 100644 (file)
--- a/libarchive/archive_read_support_format_iso9660.c
+++ b/libarchive/archive_read_support_format_iso9660.c
@@ -619,9 +619,13 @@ archive_read_format_iso9660_read_header(struct archive_read *a,
        if (file->symlink.s != NULL)
                archive_entry_copy_symlink(entry, file->symlink.s);
 
-       /* If this entry points to the same data as the previous
-        * entry, convert this into a hardlink to that entry.
-        * But don't bother for zero-length files. */
+       /* Note: If the input isn't seekable, we can't rewind to
+        * return the same body again, so if the next entry refers to
+        * the same data, we have to return it as a hardlink to the
+        * original entry. */
+       /* TODO: We have enough information here to compute an
+        * accurate value for nlinks.  We should do so and ignore
+        * nlinks from the RR extensions. */
        if (file->offset == iso9660->previous_offset
            && file->size == iso9660->previous_size
            && file->size > 0) {
@@ -634,8 +638,21 @@ archive_read_format_iso9660_read_header(struct archive_read *a,
                return (ARCHIVE_OK);
        }
 
-       /* If the offset is before our current position, we can't
-        * seek backwards to extract it, so issue a warning. */
+       /* Except for the hardlink case above, if the offset of the
+        * next entry is before our current position, we can't seek
+        * backwards to extract it, so issue a warning.  Note that
+        * this can only happen if this entry was added to the heap
+        * after we passed this offset, that is, only if the directory
+        * mentioning this entry is later than the body of the entry.
+        * Such layouts are very unusual; most ISO9660 writers lay out
+        * and record all directory information first, then store
+        * all file bodies. */
+       /* TODO: Someday, libarchive's I/O core will support optional
+        * seeking.  When that day comes, this code should attempt to
+        * seek and only return the error if the seek fails.  That
+        * will give us support for whacky ISO images that require
+        * seeking while retaining the ability to read almost all ISO
+        * images in a streaming fashion. */
        if (file->offset < iso9660->current_position) {
                archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
                    "Ignoring out-of-order file @%x (%s) %jd < %jd",
@@ -694,7 +711,7 @@ archive_read_format_iso9660_read_header(struct archive_read *a,
                                struct file_info *child;
 
                                /* N.B.: these special directory identifiers
-                                * are 8 bit "values" even on a 
+                                * are 8 bit "values" even on a
                                 * Joliet CD with UCS-2 (16bit) encoding.
                                 */
 

diff --git a/libarchive/test/test_read_format_isojoliet_bz2.c b/libarchive/test/test_read_format_isojoliet_bz2.c
index 6b442fa5825ce73f7d7f8ed371124d17a7f73e36..84ffbfe8778317ca3ee03119dd5a16faa8d68da2 100644 (file)
--- a/libarchive/test/test_read_format_isojoliet_bz2.c
+++ b/libarchive/test/test_read_format_isojoliet_bz2.c
@@ -108,10 +108,12 @@ joliettest(int withrr)
                assertEqualInt(2, archive_entry_gid(ae));
        }
 
-       /* A regular file with a hardlink. */
+       /* A regular file with two names ("hardlink" gets returned
+        * first, so it's not marked as a hardlink). */
        assertEqualInt(0, archive_read_next_header(a, &ae));
        assertEqualString("hardlink", archive_entry_pathname(ae));
        assertEqualInt(AE_IFREG, archive_entry_filetype(ae));
+       assert(archive_entry_hardlink(ae) == NULL);
        assertEqualInt(6, archive_entry_size(ae));
        assertEqualInt(0, archive_read_data_block(a, &p, &size, &offset));
        assertEqualInt(6, (int)size);
@@ -120,20 +122,27 @@ joliettest(int withrr)
        if (withrr) {
                assertEqualInt(86401, archive_entry_mtime(ae));
                assertEqualInt(86401, archive_entry_atime(ae));
-               assertEqualInt(2, archive_entry_stat(ae)->st_nlink);
+               /* TODO: Actually, libarchive should be able to
+                * compute nlinks correctly even without RR
+                * extensions. See comments in libarchive source. */
+               assertEqualInt(2, archive_entry_nlink(ae));
                assertEqualInt(1, archive_entry_uid(ae));
                assertEqualInt(2, archive_entry_gid(ae));
        }
 
-       /* Another link to the regular file. */
+       /* Second name for the same regular file (this happens to be
+        * returned second, so does get marked as a hardlink). */
        assertEqualInt(0, archive_read_next_header(a, &ae));
-       assertEqualString("long-joliet-file-name.textfile", archive_entry_pathname(ae));
+       assertEqualString("long-joliet-file-name.textfile",
+           archive_entry_pathname(ae));
        assertEqualInt(AE_IFREG, archive_entry_filetype(ae));
-       assertEqualInt(0, archive_entry_size(ae));
+       assertEqualString("hardlink", archive_entry_hardlink(ae));
+       assert(!archive_entry_size_is_set(ae));
        if (withrr) {
                assertEqualInt(86401, archive_entry_mtime(ae));
                assertEqualInt(86401, archive_entry_atime(ae));
-               assertEqualInt(2, archive_entry_stat(ae)->st_nlink);
+               /* TODO: See above. */
+               assertEqualInt(2, archive_entry_nlink(ae));
                assertEqualInt(1, archive_entry_uid(ae));
                assertEqualInt(2, archive_entry_gid(ae));
        }
@@ -150,7 +159,7 @@ joliettest(int withrr)
        assertEqualInt(172802, archive_entry_mtime(ae));
        assertEqualInt(172802, archive_entry_atime(ae));
        if (withrr) {
-               assertEqualInt(1, archive_entry_stat(ae)->st_nlink);
+               assertEqualInt(1, archive_entry_nlink(ae));
                assertEqualInt(1, archive_entry_uid(ae));
                assertEqualInt(2, archive_entry_gid(ae));
        }