From 3408183b3923e0e05811984a33260a7b80b15146 Mon Sep 17 00:00:00 2001 From: Stephen Seo Date: Mon, 9 Sep 2024 11:25:00 +0900 Subject: [PATCH] Refactor hash_map insert to use single ptr Previous implementation used a double ptr for inserting into a hash map. This refactoring allows for only needing to use a single ptr. Fixes https://git.seodisparate.com/stephenseo/SimpleArchiver/issues/17 . --- src/archiver.c | 6 +++--- src/data_structures/hash_map.c | 39 +++++++++++++++++++++------------- src/data_structures/hash_map.h | 2 +- src/data_structures/test.c | 8 +++---- src/parser.c | 4 ++-- 5 files changed, 34 insertions(+), 25 deletions(-) diff --git a/src/archiver.c b/src/archiver.c index d87a3d1..b83fd91 100644 --- a/src/archiver.c +++ b/src/archiver.c @@ -931,7 +931,7 @@ int filenames_to_abs_map_fn(void *data, void *ud) { } simple_archiver_hash_map_insert( - abs_filenames, fullpath, fullpath, strlen(fullpath) + 1, + *abs_filenames, fullpath, fullpath, strlen(fullpath) + 1, simple_archiver_helper_datastructure_cleanup_nop, NULL); // Try putting all parent dirs up to current working directory. @@ -967,7 +967,7 @@ int filenames_to_abs_map_fn(void *data, void *ud) { strncpy(fullpath_dirname_copy, fullpath_dirname, strlen(fullpath_dirname) + 1); simple_archiver_hash_map_insert( - abs_filenames, fullpath_dirname_copy, fullpath_dirname_copy, + *abs_filenames, fullpath_dirname_copy, fullpath_dirname_copy, strlen(fullpath_dirname_copy) + 1, simple_archiver_helper_datastructure_cleanup_nop, NULL); } @@ -1286,7 +1286,7 @@ int simple_archiver_parse_archive_info(FILE *in_f, int do_extract, memcpy(key, *iter, len); key[len - 1] = 0; simple_archiver_hash_map_insert( - &hash_map, key, key, len, + hash_map, key, key, len, simple_archiver_helper_datastructure_cleanup_nop, NULL); // fprintf(stderr, "\"%s\" put in map\n", key); } diff --git a/src/data_structures/hash_map.c b/src/data_structures/hash_map.c index e4b0a19..cdacdc0 100644 --- a/src/data_structures/hash_map.c +++ b/src/data_structures/hash_map.c @@ -101,12 +101,12 @@ void simple_archiver_hash_map_internal_no_free_fn( } /// Returns 0 on success. -int simple_archiver_hash_map_internal_rehash(SDArchiverHashMap **hash_map) { - if (!hash_map || !*hash_map) { +int simple_archiver_hash_map_internal_rehash(SDArchiverHashMap *hash_map) { + if (!hash_map) { return 1; } SDArchiverHashMap *new_hash_map = malloc(sizeof(SDArchiverHashMap)); - new_hash_map->buckets_size = (*hash_map)->buckets_size * 2; + new_hash_map->buckets_size = hash_map->buckets_size * 2; // Pointers have the same size (at least on the same machine), so // sizeof(void*) should be ok. new_hash_map->buckets = malloc(sizeof(void *) * new_hash_map->buckets_size); @@ -116,15 +116,14 @@ int simple_archiver_hash_map_internal_rehash(SDArchiverHashMap **hash_map) { new_hash_map->count = 0; // Iterate through the old hash map to populate the new hash map. - for (size_t bucket_idx = 0; bucket_idx < (*hash_map)->buckets_size; + for (size_t bucket_idx = 0; bucket_idx < hash_map->buckets_size; ++bucket_idx) { - SDArchiverLLNode *node = (*hash_map)->buckets[bucket_idx]->head; + SDArchiverLLNode *node = hash_map->buckets[bucket_idx]->head; while (node) { node = node->next; - if (node && node != (*hash_map)->buckets[bucket_idx]->tail && - node->data) { + if (node && node != hash_map->buckets[bucket_idx]->tail && node->data) { SDArchiverHashMapData *data = node->data; - simple_archiver_hash_map_insert(&new_hash_map, data->value, data->key, + simple_archiver_hash_map_insert(new_hash_map, data->value, data->key, data->key_size, data->value_cleanup_fn, data->key_cleanup_fn); data->key_cleanup_fn = simple_archiver_hash_map_internal_no_free_fn; @@ -133,8 +132,18 @@ int simple_archiver_hash_map_internal_rehash(SDArchiverHashMap **hash_map) { } } - simple_archiver_hash_map_free(hash_map); - *hash_map = new_hash_map; + // Free the buckets in the old hash_map. + for (size_t idx = 0; idx < hash_map->buckets_size; ++idx) { + SDArchiverLinkedList **linked_list = hash_map->buckets + idx; + simple_archiver_list_free(linked_list); + } + free(hash_map->buckets); + + // Move the new buckets and related data into the old hash_map. + *hash_map = *new_hash_map; + + // `free` the "new_hash_map" as the needed data was moved from it. + free(new_hash_map); return 0; } @@ -167,11 +176,11 @@ void simple_archiver_hash_map_free(SDArchiverHashMap **hash_map) { } } -int simple_archiver_hash_map_insert(SDArchiverHashMap **hash_map, void *value, +int simple_archiver_hash_map_insert(SDArchiverHashMap *hash_map, void *value, void *key, size_t key_size, void (*value_cleanup_fn)(void *), void (*key_cleanup_fn)(void *)) { - if ((*hash_map)->buckets_size <= (*hash_map)->count) { + if (hash_map->buckets_size <= hash_map->count) { simple_archiver_hash_map_internal_rehash(hash_map); } @@ -184,13 +193,13 @@ int simple_archiver_hash_map_insert(SDArchiverHashMap **hash_map, void *value, unsigned long long hash = simple_archiver_hash_map_internal_key_to_hash(key, key_size) % - (*hash_map)->buckets_size; + hash_map->buckets_size; int result = simple_archiver_list_add_front( - (*hash_map)->buckets[hash], data, + hash_map->buckets[hash], data, simple_archiver_hash_map_internal_cleanup_data); if (result == 0) { - ++(*hash_map)->count; + ++hash_map->count; return 0; } else { if (value) { diff --git a/src/data_structures/hash_map.h b/src/data_structures/hash_map.h index c9e735f..41e0087 100644 --- a/src/data_structures/hash_map.h +++ b/src/data_structures/hash_map.h @@ -41,7 +41,7 @@ void simple_archiver_hash_map_free(SDArchiverHashMap **hash_map); /// If key_cleanup_fn is NULL, then "free" is used instead. /// NOTICE: You must not pass NULL to value, otherwise all "get" checks will /// fail for the inserted key. -int simple_archiver_hash_map_insert(SDArchiverHashMap **hash_map, void *value, +int simple_archiver_hash_map_insert(SDArchiverHashMap *hash_map, void *value, void *key, size_t key_size, void (*value_cleanup_fn)(void *), void (*key_cleanup_fn)(void *)); diff --git a/src/data_structures/test.c b/src/data_structures/test.c index 3303191..0144553 100644 --- a/src/data_structures/test.c +++ b/src/data_structures/test.c @@ -145,8 +145,8 @@ int main(void) { key = malloc(sizeof(int)); *value = idx; *key = idx; - simple_archiver_hash_map_insert(&hash_map, value, key, sizeof(int), - NULL, NULL); + simple_archiver_hash_map_insert(hash_map, value, key, sizeof(int), NULL, + NULL); } } @@ -184,7 +184,7 @@ int main(void) { *copy_value = idx; unsigned int *copy_key = malloc(sizeof(unsigned int)); *copy_key = idx; - simple_archiver_hash_map_insert(&hash_map, copy_value, copy_key, + simple_archiver_hash_map_insert(hash_map, copy_value, copy_key, sizeof(unsigned int), NULL, NULL); } simple_archiver_hash_map_free(&hash_map); @@ -193,7 +193,7 @@ int main(void) { hash_map = simple_archiver_hash_map_init(); for (size_t idx = 0; idx < SDARCHIVER_DS_TEST_HASH_MAP_ITER_SIZE; ++idx) { - simple_archiver_hash_map_insert(&hash_map, (void *)idx, &idx, + simple_archiver_hash_map_insert(hash_map, (void *)idx, &idx, sizeof(size_t), no_free_fn, no_free_fn); } diff --git a/src/parser.c b/src/parser.c index 4807751..ce3c17b 100644 --- a/src/parser.c +++ b/src/parser.c @@ -442,7 +442,7 @@ SDArchiverLinkedList *simple_archiver_parsed_to_filenames( simple_archiver_list_add(files_list, file_info, simple_archiver_internal_free_file_info_fn); simple_archiver_hash_map_insert( - &hash_map, &hash_map_sentinel, filename, len - 1, + hash_map, &hash_map_sentinel, filename, len - 1, simple_archiver_helper_datastructure_cleanup_nop, simple_archiver_helper_datastructure_cleanup_nop); } else { @@ -541,7 +541,7 @@ SDArchiverLinkedList *simple_archiver_parsed_to_filenames( files_list, file_info, simple_archiver_internal_free_file_info_fn); simple_archiver_hash_map_insert( - &hash_map, &hash_map_sentinel, combined_path, + hash_map, &hash_map_sentinel, combined_path, combined_size - 1, simple_archiver_helper_datastructure_cleanup_nop, simple_archiver_helper_datastructure_cleanup_nop);