More comments
authorEric Biggers <ebiggers3@gmail.com>
Thu, 23 Aug 2012 21:57:02 +0000 (16:57 -0500)
committerEric Biggers <ebiggers3@gmail.com>
Thu, 23 Aug 2012 21:57:02 +0000 (16:57 -0500)
Also calculate hash in write_wim_resource_from_buffer() and not
write_metadata_resource() to avoid redundant SHA1 calculations

src/dentry.c
src/resource.c
src/sha1.c
src/sha1.h

index 27937ab42a55a933e6833e34d5325bec7fdbd757..c07ab5ea5504672c28a4c9645118921ebe58c734 100644 (file)
@@ -59,7 +59,7 @@ static bool dentry_has_name(const struct dentry *dentry, const char *name,
 u64 dentry_total_length(const struct dentry *dentry)
 {
        u64 length = (dentry->length + 7) & ~7;
-       for (u16 i = 0 ; i < dentry->num_ads; i++)
+       for (u16 i = 0; i < dentry->num_ads; i++)
                length += ads_entry_length(&dentry->ads_entries[i]);
        return length;
 }
index 00dc9d0ea6432556e38ac407a77ca47967b01f94..4f093622f9e64d356484015f8b45db1cc33635b9 100644 (file)
@@ -702,6 +702,11 @@ finish_wim_resource_chunk_tab(struct chunk_table *chunk_tab,
  * Writes a WIM resource to a FILE * opened for writing.  The resource may be
  * written uncompressed or compressed depending on the @out_ctype parameter.
  *
+ * If by chance the resource compresses to more than the original size (this may
+ * happen with random data or files than are pre-compressed), the resource is
+ * instead written uncompressed (and this is reflected in the @out_res_entry by
+ * removing the WIM_RESHDR_FLAG_COMPRESSED flag).
+ *
  * @lte:       The lookup table entry for the WIM resource.
  * @out_fp:    The FILE * to write the resource to.
  * @out_ctype:  The compression type of the resource to write.  Note: if this is
@@ -728,9 +733,13 @@ static int write_wim_resource(struct lookup_table_entry *lte,
        bool raw;
        off_t file_offset;
 
+       /* Original size of the resource */
        original_size = wim_resource_size(lte);
+
+       /* Compressed size of the resource (as it exists now) */
        old_compressed_size = wim_resource_compressed_size(lte);
 
+       /* Current offset in output file */
        file_offset = ftello(out_fp);
        if (file_offset == -1) {
                ERROR_WITH_ERRNO("Failed to get offset in output "
@@ -738,6 +747,8 @@ static int write_wim_resource(struct lookup_table_entry *lte,
                return WIMLIB_ERR_WRITE;
        }
        
+       /* Are the compression types the same?  If so, do a raw copy (copy
+        * without decompressing and recompressing the data). */
        raw = (wim_resource_compression_type(lte) == out_ctype
               && out_ctype != WIM_COMPRESSION_TYPE_NONE);
        if (raw)
@@ -745,11 +756,15 @@ static int write_wim_resource(struct lookup_table_entry *lte,
        else
                bytes_remaining = original_size;
 
+       /* Empty resource; nothing needs to be done, so just return success. */
        if (bytes_remaining == 0)
                return 0;
 
+       /* Buffer for reading chunks for the resource */
        char buf[min(WIM_CHUNK_SIZE, bytes_remaining)];
 
+       /* If we are writing a compressed resource and not doing a raw copy, we
+        * need to initialize the chunk table */
        if (out_ctype != WIM_COMPRESSION_TYPE_NONE && !raw) {
                ret = begin_wim_resource_chunk_tab(lte, out_fp, file_offset,
                                                   &chunk_tab);
@@ -757,11 +772,12 @@ static int write_wim_resource(struct lookup_table_entry *lte,
                        goto out;
        }
 
+       /* If the WIM resource is in an external file, open a FILE * to it so we
+        * don't have to open a temporary one in read_wim_resource() for each
+        * chunk. */
        if (lte->resource_location == RESOURCE_IN_FILE_ON_DISK
             && !lte->file_on_disk_fp)
        {
-               /* The WIM resource is in an external file; open a FILE * to it
-                * so we don't have to open a temporary one on every read. */
                wimlib_assert(lte->file_on_disk);
                lte->file_on_disk_fp = fopen(lte->file_on_disk, "rb");
                if (!lte->file_on_disk_fp) {
@@ -771,10 +787,18 @@ static int write_wim_resource(struct lookup_table_entry *lte,
                        goto out;
                }
        }
+
+       /* If we aren't doing a raw copy, we will compute the SHA1 message
+        * digest of the resource as we read it, and verify it's the same as the
+        * hash given in the lookup table entry once we've finished reading the
+        * resource. */
        SHA_CTX ctx;
        if (!raw)
                sha1_init(&ctx);
 
+       /* While there are still bytes remaining in the WIM resource, read a
+        * chunk of the resource, update SHA1, then write that chunk using the
+        * desired compression type. */
        do {
                u64 to_read = min(bytes_remaining, WIM_CHUNK_SIZE);
                ret = read_wim_resource(lte, buf, to_read, offset, raw);
@@ -789,6 +813,12 @@ static int write_wim_resource(struct lookup_table_entry *lte,
                bytes_remaining -= to_read;
                offset += to_read;
        } while (bytes_remaining);
+
+       /* If writing a compressed resource and not doing a raw copy, write the
+        * chunk table, and finish_wim_resource_chunk_tab() will provide the
+        * compressed size of the resource we wrote.  Otherwise, the compressed
+        * size of the written resource is the same as the compressed size of
+        * the existing resource. */
        if (out_ctype != WIM_COMPRESSION_TYPE_NONE && !raw) {
                ret = finish_wim_resource_chunk_tab(chunk_tab, out_fp,
                                                    &new_compressed_size);
@@ -798,13 +828,16 @@ static int write_wim_resource(struct lookup_table_entry *lte,
                new_compressed_size = old_compressed_size;
        }
 
+       /* Verify SHA1 message digest of the resource, unless we are doing a raw
+        * write (in which case we never even saw the uncompressed data).  Or,
+        * if the hash we had before is all 0's, just re-set it to be the new
+        * hash. */
        if (!raw) {
-               /* Verify SHA1 message digest of the resource, unless we are
-                * doing a raw write (in which case we may have never even seen
-                * the uncompressed data)  */
                u8 md[SHA1_HASH_SIZE];
                sha1_final(md, &ctx);
-               if (!hashes_equal(md, lte->hash)) {
+               if (is_zero_hash(lte->hash)) {
+                       copy_hash(lte->hash, md);
+               } else if (!hashes_equal(md, lte->hash)) {
                        ERROR("WIM resource has incorrect hash!");
                        if (lte->resource_location == RESOURCE_IN_FILE_ON_DISK) {
                                ERROR("We were reading it from `%s'; maybe it changed "
@@ -861,20 +894,31 @@ out:
        return ret;
 }
 
+/* Like write_wim_resource(), but the resource is specified by a buffer of
+ * uncompressed data rather a lookup table entry; also writes the SHA1 hash of
+ * the buffer to @hash.  */
 static int write_wim_resource_from_buffer(const u8 *buf, u64 buf_size,
-                                         u8 buf_hash[SHA1_HASH_SIZE],
                                          FILE *out_fp, int out_ctype,
-                                         struct resource_entry *out_res_entry)
+                                         struct resource_entry *out_res_entry,
+                                         u8 hash[SHA1_HASH_SIZE])
 {
+       /* Set up a temporary lookup table entry that we provide to
+        * write_wim_resource(). */
        struct lookup_table_entry lte;
+       int ret;
        lte.resource_entry.flags         = 0;
        lte.resource_entry.original_size = buf_size;
        lte.resource_entry.size          = buf_size;
        lte.resource_entry.offset        = 0;
        lte.resource_location            = RESOURCE_IN_ATTACHED_BUFFER;
        lte.attached_buffer              = (u8*)buf;
-       copy_hash(lte.hash, buf_hash);
-       return write_wim_resource(&lte, out_fp, out_ctype, out_res_entry);
+
+       zero_hash(lte.hash);
+       ret = write_wim_resource(&lte, out_fp, out_ctype, out_res_entry);
+       if (ret != 0)
+               return ret;
+       copy_hash(hash, lte.hash);
+       return 0;
 }
 
 /* 
@@ -931,7 +975,8 @@ int extract_full_wim_resource_to_fd(const struct lookup_table_entry *lte, int fd
 
 /* 
  * Copies the file resource specified by the lookup table entry @lte from the
- * input WIM the output WIM.
+ * input WIM to the output WIM that has its FILE * given by
+ * ((WIMStruct*)wim)->out_fp.
  *
  * The output_resource_entry, out_refcnt, and part_number fields of @lte are
  * updated.
@@ -1125,30 +1170,20 @@ out_free_buf:
        return ret;
 }
 
-/* Write the metadata resource for the current image. */
+/* Write the metadata resource for the current WIM image. */
 int write_metadata_resource(WIMStruct *w)
 {
-       FILE *out;
        u8 *buf;
        u8 *p;
        int ret;
        u64 subdir_offset;
        struct dentry *root;
        struct lookup_table_entry *lte;
-       off_t metadata_offset;
        u64 metadata_original_size;
-       u64 metadata_compressed_size;
-       int metadata_ctype;
-       u8  hash[SHA1_HASH_SIZE];
 
        DEBUG("Writing metadata resource for image %d", w->current_image);
 
-       out = w->out_fp;
        root = wim_root_dentry(w);
-       metadata_ctype = wimlib_get_compression_type(w);
-       metadata_offset = ftello(out);
-       if (metadata_offset == -1)
-               return WIMLIB_ERR_WRITE;
 
        struct wim_security_data *sd = wim_security_data(w);
        if (sd)
@@ -1168,27 +1203,20 @@ int write_metadata_resource(WIMStruct *w)
 
        DEBUG("Writing dentry tree.");
        p = write_dentry_tree(root, p);
-
-       /* Like file resources, the lookup table entry for a metadata resource
-        * uses for the hash code a SHA1 message digest of its uncompressed
-        * contents. */
-       sha1_buffer(buf, metadata_original_size, hash);
-
+       wimlib_assert(p - buf == metadata_original_size);
 
        lte = wim_metadata_lookup_table_entry(w);
 
        ret = write_wim_resource_from_buffer(buf, metadata_original_size,
-                                            hash, out, metadata_ctype,
-                                            &lte->output_resource_entry);
+                                            w->out_fp,
+                                            wimlib_get_compression_type(w),
+                                            &lte->output_resource_entry,
+                                            lte->hash);
 
        lookup_table_unlink(w->lookup_table, lte);
-       copy_hash(lte->hash, hash);
        lookup_table_insert(w->lookup_table, lte);
        lte->out_refcnt++;
        lte->output_resource_entry.flags |= WIM_RESHDR_FLAG_METADATA;
        FREE(buf);
-       if (ret != 0)
-               return ret;
-
-       return 0;
+       return ret;
 }
index dd4e26293040f0860b2dfcdb6ccfbaa9d341c88a..27e0daf3a4badc7d449cd575f61bc83dbe152ff5 100644 (file)
@@ -179,7 +179,7 @@ void sha1_final(u8 md[SHA1_HASH_SIZE], SHA_CTX* context)
        while ((context->count[0] & 504) != 448) {
                sha1_update(context, (u8 *)"\0", 1);
        }
-       sha1_update(context, finalcount, 8);  /* Should cause a SHA1_Transform() */
+       sha1_update(context, finalcount, 8);  /* Should cause a sha1_transform() */
        for (i = 0; i < SHA1_HASH_SIZE; i++) {
                md[i] = (u8)((context->state[i>>2] >> ((3-(i & 3)) * 8) ) & 255);
        }
index 5c1436170f8b2b79b6beb3098dd44bf36d737002..c44b9f40f76ce46cd78d7c12eec3c86a17d28498 100644 (file)
@@ -27,11 +27,24 @@ static inline bool hashes_equal(const u8 h1[SHA1_HASH_SIZE],
 }
 
 /* Prints a hash code field. */
-static inline void print_hash(const u8 hash[])
+static inline void print_hash(const u8 hash[SHA1_HASH_SIZE])
 {
        print_byte_field(hash, SHA1_HASH_SIZE);
 }
 
+static inline bool is_zero_hash(const u8 hash[SHA1_HASH_SIZE])
+{
+       for (u8 i = 0; i < SHA1_HASH_SIZE / 4; i++)
+               if (((u32*)hash)[i])
+                       return false;
+       return true;
+}
+
+static void zero_hash(u8 hash[SHA1_HASH_SIZE])
+{
+       memset(hash, 0, SHA1_HASH_SIZE);
+}
+
 
 #ifdef WITH_LIBCRYPTO