From: Eric Biggers Date: Wed, 22 May 2013 05:45:36 +0000 (-0500) Subject: Remove verify_dentry(); separate refcnt recalc. from verify_inode() X-Git-Tag: v1.4.1~32 X-Git-Url: https://wimlib.net/git/?p=wimlib;a=commitdiff_plain;h=fced769402ff6ef0bf75fe9e4dcb2880891469e0;hp=1cf955f8e732e1745a90457e652505f24b7f8cf5 Remove verify_dentry(); separate refcnt recalc. from verify_inode() --- diff --git a/include/wimlib/dentry.h b/include/wimlib/dentry.h index 282cad45..dfb5a95b 100644 --- a/include/wimlib/dentry.h +++ b/include/wimlib/dentry.h @@ -18,6 +18,7 @@ struct wim_lookup_table; struct wim_lookup_table_entry; struct wimfs_fd; struct wim_inode; +struct wim_security_data; /* Size of the struct wim_dentry up to and including the file_name_len. */ #define WIM_DENTRY_DISK_SIZE 102 @@ -252,9 +253,6 @@ struct wim_inode { * (This is not an on-disk field.) */ u8 i_resolved : 1; - /* %true iff verify_inode() has run on this inode. */ - u8 i_verified : 1; - u8 i_visited : 1; /* Used only in NTFS-mode extraction */ @@ -351,6 +349,9 @@ struct wim_inode { #define inode_first_dentry(inode) \ container_of(inode->i_dentry.next, struct wim_dentry, d_alias) +#define inode_first_full_path(inode) \ + dentry_full_path(inode_first_dentry(inode)) + static inline bool dentry_is_first_in_inode(const struct wim_dentry *dentry) { @@ -601,7 +602,7 @@ inode_ref_streams(struct wim_inode *inode); extern int dentry_tree_fix_inodes(struct wim_dentry *root, struct list_head *inode_list); -extern int -verify_dentry(struct wim_dentry *dentry, void *wim); +int +verify_inode(struct wim_inode *inode, const struct wim_security_data *sd); #endif diff --git a/include/wimlib/error.h b/include/wimlib/error.h index 7e0a7ced..8dd48c18 100644 --- a/include/wimlib/error.h +++ b/include/wimlib/error.h @@ -45,7 +45,9 @@ wimlib_warning_with_errno(const tchar *format, ...) # define ERROR_WITH_ERRNO(format, ...) wimlib_error_with_errno(T(format), ## __VA_ARGS__) # define WARNING(format, ...) wimlib_warning(T(format), ## __VA_ARGS__) # define WARNING_WITH_ERRNO(format, ...) wimlib_warning_with_errno(T(format), ## __VA_ARGS__) +extern bool wimlib_print_errors; #else /* ENABLE_ERROR_MESSAGES */ +# define wimlib_print_errors 0 # define ERROR(format, ...) dummy_tprintf(T(format), ## __VA_ARGS__) # define ERROR_WITH_ERRNO(format, ...) dummy_tprintf(T(format), ## __VA_ARGS__) # define WARNING(format, ...) dummy_tprintf(T(format), ## __VA_ARGS__) @@ -60,6 +62,7 @@ wimlib_warning_with_errno(const tchar *format, ...) # define ENABLE_ASSERTIONS 1 #endif + #ifdef ENABLE_DEBUG extern void wimlib_debug(const tchar *file, int line, const char *func, diff --git a/include/wimlib/wim.h b/include/wimlib/wim.h index 9d01522a..a2d664ea 100644 --- a/include/wimlib/wim.h +++ b/include/wimlib/wim.h @@ -47,12 +47,18 @@ struct WIMStruct { /* Have any images been deleted? */ u8 deletion_occurred : 1; - u8 all_images_verified : 1; + /* Do we know that all the stream reference counts in the WIM are + * correct? If so, this is set to 1 and deletions are safe; otherwise + * this is set to 0 and deletions are not safe until reference counts + * are recalculated. (This is due to a bug in M$'s software that + * generates WIMs with invalid reference counts.) */ + u8 refcnts_ok : 1; + u8 wim_locked : 1; }; -extern int -wim_run_full_verifications(WIMStruct *w); +extern void +wim_recalculate_refcnts(WIMStruct *wim); extern int read_header(const tchar *filename, int in_fd, struct wim_header *hdr); diff --git a/src/hardlink.c b/src/hardlink.c index 23ae370f..461c9523 100644 --- a/src/hardlink.c +++ b/src/hardlink.c @@ -209,25 +209,23 @@ inode_table_new_dentry(struct wim_inode_table *table, const tchar *name, return 0; } -#if defined(ENABLE_ERROR_MESSAGES) || defined(ENABLE_DEBUG) -static void +static inline void print_inode_dentries(const struct wim_inode *inode) { struct wim_dentry *dentry; inode_for_each_dentry(dentry, inode) tfprintf(stderr, T("%"TS"\n"), dentry_full_path(dentry)); } -#endif static void inconsistent_inode(const struct wim_inode *inode) { -#ifdef ENABLE_ERROR_MESSAGES - ERROR("An inconsistent hard link group that cannot be corrected has " - "been detected"); - ERROR("The dentries are located at the following paths:"); - print_inode_dentries(inode); -#endif + if (wimlib_print_errors) { + ERROR("An inconsistent hard link group that cannot be corrected has " + "been detected"); + ERROR("The dentries are located at the following paths:"); + print_inode_dentries(inode); + } } static bool diff --git a/src/lookup_table.c b/src/lookup_table.c index d225d627..955aac35 100644 --- a/src/lookup_table.c +++ b/src/lookup_table.c @@ -463,12 +463,12 @@ read_lookup_table(WIMStruct *w) && (cur_entry->resource_entry.size != cur_entry->resource_entry.original_size)) { - #ifdef ENABLE_ERROR_MESSAGES - ERROR("Found uncompressed resource with original size " - "not the same as compressed size"); - ERROR("The lookup table entry for the resource is as follows:"); - print_lookup_table_entry(cur_entry, stderr); - #endif + if (wimlib_print_errors) { + ERROR("Found uncompressed resource with original size " + "not the same as compressed size"); + ERROR("The lookup table entry for the resource is as follows:"); + print_lookup_table_entry(cur_entry, stderr); + } ret = WIMLIB_ERR_INVALID_LOOKUP_TABLE_ENTRY; goto out_free_cur_entry; } @@ -476,10 +476,10 @@ read_lookup_table(WIMStruct *w) if (cur_entry->resource_entry.flags & WIM_RESHDR_FLAG_METADATA) { /* Lookup table entry for a metadata resource */ if (cur_entry->refcnt != 1) { - #ifdef ENABLE_ERROR_MESSAGES - ERROR("Found metadata resource with refcnt != 1:"); - print_lookup_table_entry(cur_entry, stderr); - #endif + if (wimlib_print_errors) { + ERROR("Found metadata resource with refcnt != 1:"); + print_lookup_table_entry(cur_entry, stderr); + } ret = WIMLIB_ERR_INVALID_LOOKUP_TABLE_ENTRY; goto out_free_cur_entry; } @@ -516,14 +516,14 @@ read_lookup_table(WIMStruct *w) * metadata resource */ duplicate_entry = __lookup_resource(table, cur_entry->hash); if (duplicate_entry) { - #ifdef ENABLE_ERROR_MESSAGES - ERROR("The WIM lookup table contains two entries with the " - "same SHA1 message digest!"); - ERROR("The first entry is:"); - print_lookup_table_entry(duplicate_entry, stderr); - ERROR("The second entry is:"); - print_lookup_table_entry(cur_entry, stderr); - #endif + if (wimlib_print_errors) { + ERROR("The WIM lookup table contains two entries with the " + "same SHA1 message digest!"); + ERROR("The first entry is:"); + print_lookup_table_entry(duplicate_entry, stderr); + ERROR("The second entry is:"); + print_lookup_table_entry(cur_entry, stderr); + } ret = WIMLIB_ERR_INVALID_LOOKUP_TABLE_ENTRY; goto out_free_cur_entry; } diff --git a/src/metadata_resource.c b/src/metadata_resource.c index e600f301..5f392e52 100644 --- a/src/metadata_resource.c +++ b/src/metadata_resource.c @@ -60,6 +60,8 @@ read_metadata_resource(WIMStruct *wim, struct wim_image_metadata *imd) const struct wim_lookup_table_entry *metadata_lte; u64 metadata_len; u8 hash[SHA1_HASH_SIZE]; + struct wim_security_data *security_data; + struct wim_inode *inode; metadata_lte = imd->metadata_lte; metadata_len = wim_resource_size(metadata_lte); @@ -121,7 +123,7 @@ read_metadata_resource(WIMStruct *wim, struct wim_image_metadata *imd) * and calculate the offset in the metadata resource of the root dentry. * */ - ret = read_wim_security_data(buf, metadata_len, &imd->security_data); + ret = read_wim_security_data(buf, metadata_len, &security_data); if (ret) goto out_free_buf; @@ -135,7 +137,7 @@ read_metadata_resource(WIMStruct *wim, struct wim_image_metadata *imd) } ret = read_dentry(buf, metadata_len, - imd->security_data->total_length, root); + security_data->total_length, root); if (ret == 0 && root->length == 0) { WARNING("Metadata resource begins with end-of-directory entry " @@ -176,27 +178,24 @@ read_metadata_resource(WIMStruct *wim, struct wim_image_metadata *imd) if (ret) goto out_free_dentry_tree; - if (!wim->all_images_verified) { - /* Note: verify_dentry() expects to access imd->security_data, - * so it needs to be set before here. */ - DEBUG("Running miscellaneous verifications on the dentry tree"); - for_lookup_table_entry(wim->lookup_table, lte_zero_real_refcnt, NULL); - ret = for_dentry_in_tree(root, verify_dentry, wim); + + DEBUG("Running miscellaneous verifications on the dentry tree"); + image_for_each_inode(inode, imd) { + ret = verify_inode(inode, security_data); if (ret) goto out_free_dentry_tree; } - DEBUG("Done reading image metadata"); - out_success: imd->root_dentry = root; + imd->security_data = security_data; INIT_LIST_HEAD(&imd->unhashed_streams); + ret = 0; goto out_free_buf; out_free_dentry_tree: free_dentry_tree(root, wim->lookup_table); out_free_security_data: - free_wim_security_data(imd->security_data); - imd->security_data = NULL; + free_wim_security_data(security_data); out_free_buf: FREE(buf); return ret; diff --git a/src/resource.c b/src/resource.c index 29bfcd7f..f37b8958 100644 --- a/src/resource.c +++ b/src/resource.c @@ -691,13 +691,13 @@ extract_wim_resource(const struct wim_lookup_table_entry *lte, u8 hash[SHA1_HASH_SIZE]; sha1_final(hash, &ctx.sha_ctx); if (!hashes_equal(hash, lte->hash)) { - #ifdef ENABLE_ERROR_MESSAGES - ERROR("Invalid SHA1 message digest " - "on the following WIM resource:"); - print_lookup_table_entry(lte, stderr); - if (lte->resource_location == RESOURCE_IN_WIM) - ERROR("The WIM file appears to be corrupt!"); - #endif + if (wimlib_print_errors) { + ERROR("Invalid SHA1 message digest " + "on the following WIM resource:"); + print_lookup_table_entry(lte, stderr); + if (lte->resource_location == RESOURCE_IN_WIM) + ERROR("The WIM file appears to be corrupt!"); + } ret = WIMLIB_ERR_INVALID_RESOURCE_HASH; } } diff --git a/src/util.c b/src/util.c index 2aca3da7..f5a8ca29 100644 --- a/src/util.c +++ b/src/util.c @@ -145,7 +145,7 @@ wimlib_fprintf(FILE *fp, const tchar *format, ...) #endif /* __WIN32__ */ #ifdef ENABLE_ERROR_MESSAGES -static bool wimlib_print_errors = false; +bool wimlib_print_errors = false; #endif #if defined(ENABLE_ERROR_MESSAGES) || defined(ENABLE_DEBUG) diff --git a/src/verify.c b/src/verify.c index 501aea60..3702ee7e 100644 --- a/src/verify.c +++ b/src/verify.c @@ -1,8 +1,7 @@ /* * verify.c * - * Some functions to verify that stuff in the WIM is valid. Of course, not - * *all* the verifications of the input data are in this file. + * Verify WIM inodes and stream reference counts. */ /* @@ -34,12 +33,16 @@ #include "wimlib/metadata.h" #include "wimlib/security.h" -static int -verify_inode(struct wim_inode *inode, const WIMStruct *w) +/* + * Verify a WIM inode: + * + * - Check to make sure the security ID is valid + * - Check to make sure there is at most one unnamed stream + * - Check to make sure there is at most one DOS name. + */ +int +verify_inode(struct wim_inode *inode, const struct wim_security_data *sd) { - const struct wim_lookup_table *table = w->lookup_table; - const struct wim_security_data *sd = wim_const_security_data(w); - struct wim_dentry *first_dentry = inode_first_dentry(inode); struct wim_dentry *dentry; /* Check the security ID. -1 is valid and means "no security @@ -50,32 +53,10 @@ verify_inode(struct wim_inode *inode, const WIMStruct *w) inode->i_security_id >= sd->num_entries)) { WARNING("\"%"TS"\" has an invalid security ID (%d)", - dentry_full_path(first_dentry), inode->i_security_id); + inode_first_full_path(inode), inode->i_security_id); inode->i_security_id = -1; } - /* Check that lookup table entries for all the inode's stream exist, - * except if the SHA1 message digest is all 0's, which indicates an - * empty stream. - * - * This check is skipped on split WIMs. */ - if (w->hdr.total_parts == 1 && !inode->i_resolved) { - for (unsigned i = 0; i <= inode->i_num_ads; i++) { - struct wim_lookup_table_entry *lte; - const u8 *hash; - hash = inode_stream_hash(inode, i); - lte = __lookup_resource(table, hash); - if (!lte && !is_zero_hash(hash)) { - ERROR("Could not find lookup table entry for stream " - "%u of dentry `%"TS"'", - i, dentry_full_path(first_dentry)); - return WIMLIB_ERR_INVALID_DENTRY; - } - if (lte) - lte->real_refcnt += inode->i_nlink; - } - } - /* Make sure there is only one unnamed data stream. */ unsigned num_unnamed_streams = 0; for (unsigned i = 0; i <= inode->i_num_ads; i++) { @@ -86,7 +67,7 @@ verify_inode(struct wim_inode *inode, const WIMStruct *w) } if (num_unnamed_streams > 1) { WARNING("\"%"TS"\" has multiple (%u) un-named streams", - dentry_full_path(first_dentry), num_unnamed_streams); + inode_first_full_path(inode), num_unnamed_streams); } /* Files cannot have multiple DOS names, even if they have multiple @@ -113,57 +94,52 @@ verify_inode(struct wim_inode *inode, const WIMStruct *w) dentry_with_dos_name = dentry; } } - - inode->i_verified = 1; return 0; } -/* Run some miscellaneous verifications on a WIM dentry */ -int -verify_dentry(struct wim_dentry *dentry, void *wim) +static int +lte_fix_refcnt(struct wim_lookup_table_entry *lte, void *ctr) { - int ret; - WIMStruct *w = wim; - /* Verify the associated inode, but only one time no matter how many - * dentries it has (unless we are doing a full verification of the WIM, - * in which case we need to force the inode to be verified again.) */ - if (!dentry->d_inode->i_verified) { - ret = verify_inode(dentry->d_inode, w); - if (ret) - return ret; + if (lte->refcnt != lte->real_refcnt) { + if (wimlib_print_errors) { + WARNING("The following lookup table entry has a reference " + "count of %u, but", lte->refcnt); + WARNING("We found %u references to it", + lte->real_refcnt); + print_lookup_table_entry(lte, stderr); + } + lte->refcnt = lte->real_refcnt; + ++*(unsigned long *)ctr; } return 0; } -static int -image_run_full_verifications(WIMStruct *w) +static void +tally_inode_refcnts(const struct wim_inode *inode, + const struct wim_lookup_table *lookup_table) { - struct wim_image_metadata *imd; - struct wim_inode *inode; - - imd = wim_get_current_image_metadata(w); - image_for_each_inode(inode, imd) { - inode->i_verified = 0; - return for_dentry_in_tree(imd->root_dentry, verify_dentry, w); + for (unsigned i = 0; i <= inode->i_num_ads; i++) { + struct wim_lookup_table_entry *lte; + lte = inode_stream_lte(inode, i, lookup_table); + if (lte) + lte->real_refcnt += inode->i_nlink; + } } + static int -lte_fix_refcnt(struct wim_lookup_table_entry *lte, void *ctr) +tally_image_refcnts(WIMStruct *wim) { - if (lte->refcnt != lte->real_refcnt) { - #ifdef ENABLE_ERROR_MESSAGES - WARNING("The following lookup table entry has a reference " - "count of %u, but", lte->refcnt); - WARNING("We found %u references to it", - lte->real_refcnt); - print_lookup_table_entry(lte, stderr); - #endif - lte->refcnt = lte->real_refcnt; - ++*(unsigned long *)ctr; - } + const struct wim_image_metadata *imd; + const struct wim_inode *inode; + + imd = wim_get_current_image_metadata(wim); + image_for_each_inode(inode, imd) + tally_inode_refcnts(inode, wim->lookup_table); return 0; } + /* Ideally this would be unnecessary... however, the WIMs for Windows 8 are * screwed up because some lookup table entries are referenced more times than * their stated reference counts. So theoretically, if we delete all the @@ -172,28 +148,21 @@ lte_fix_refcnt(struct wim_lookup_table_entry *lte, void *ctr) * problem by looking at ALL the images to re-calculate the reference count of * EVERY lookup table entry. This only absolutely has to be done before an image * is deleted or before an image is mounted read-write. */ -int -wim_run_full_verifications(WIMStruct *w) +void +wim_recalculate_refcnts(WIMStruct *wim) { - int ret; - - for_lookup_table_entry(w->lookup_table, lte_zero_real_refcnt, NULL); - - w->all_images_verified = 1; /* Set *before* image_run_full_verifications, - because of check in read_metadata_resource() */ - ret = for_image(w, WIMLIB_ALL_IMAGES, image_run_full_verifications); - if (ret == 0) { - unsigned long num_ltes_with_bogus_refcnt = 0; - for_lookup_table_entry(w->lookup_table, lte_fix_refcnt, - &num_ltes_with_bogus_refcnt); - if (num_ltes_with_bogus_refcnt != 0) { - WARNING("A total of %lu entries in the WIM's stream " - "lookup table had to have\n" - " their reference counts fixed.", - num_ltes_with_bogus_refcnt); - } - } else { - w->all_images_verified = 0; + unsigned long num_ltes_with_bogus_refcnt = 0; + + for_lookup_table_entry(wim->lookup_table, lte_zero_real_refcnt, NULL); + for_image(wim, WIMLIB_ALL_IMAGES, tally_image_refcnts); + num_ltes_with_bogus_refcnt = 0; + for_lookup_table_entry(wim->lookup_table, lte_fix_refcnt, + &num_ltes_with_bogus_refcnt); + if (num_ltes_with_bogus_refcnt != 0) { + WARNING("A total of %lu entries in the WIM's stream " + "lookup table had to have\n" + " their reference counts fixed.", + num_ltes_with_bogus_refcnt); } - return ret; + wim->refcnts_ok = 1; } diff --git a/src/wim.c b/src/wim.c index 7927af3c..dec3ee96 100644 --- a/src/wim.c +++ b/src/wim.c @@ -159,6 +159,7 @@ wimlib_create_new_wim(int ctype, WIMStruct **w_ret) goto out_free; } w->lookup_table = table; + w->refcnts_ok = 1; *w_ret = w; return 0; out_free: @@ -740,9 +741,11 @@ can_delete_from_wim(WIMStruct *wim) int ret; ret = can_modify_wim(wim); - if (ret == 0 && !wim->all_images_verified) - ret = wim_run_full_verifications(wim); - return ret; + if (ret) + return ret; + if (!wim->refcnts_ok) + wim_recalculate_refcnts(wim); + return 0; } /* Frees the memory for the WIMStruct, including all internal memory; also diff --git a/src/write.c b/src/write.c index 2effd7db..ba8b6970 100644 --- a/src/write.c +++ b/src/write.c @@ -1547,10 +1547,10 @@ lte_overwrite_prepare_2(struct wim_lookup_table_entry *lte, void *_args) if (lte->resource_entry.offset + lte->resource_entry.size > args->end_offset) { - #ifdef ENABLE_ERROR_MESSAGES - ERROR("The following resource is after the XML data:"); - print_lookup_table_entry(lte, stderr); - #endif + if (wimlib_print_errors) { + ERROR("The following resource is after the XML data:"); + print_lookup_table_entry(lte, stderr); + } return WIMLIB_ERR_RESOURCE_ORDER; } copy_resource_entry(<e->output_resource_entry,