diff --git a/builtin/rev-list.c b/builtin/rev-list.c index ddea8aa251..ab5f69826c 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -88,9 +88,19 @@ static int arg_print_omitted; /* print objects omitted by filter */ struct missing_objects_map_entry { struct oidmap_entry entry; - const char *path; + char *path; unsigned type; }; + +static void free_missing_objects_entry(void *e) +{ + struct missing_objects_map_entry *entry = + container_of(e, struct missing_objects_map_entry, entry); + + free(entry->path); + free(entry); +} + static struct oidmap missing_objects; enum missing_action { MA_ERROR = 0, /* fail if any missing objects are encountered */ @@ -935,10 +945,9 @@ int cmd_rev_list(int argc, while ((entry = oidmap_iter_next(&iter))) { print_missing_object(entry, arg_missing_action == MA_PRINT_INFO); - free((void *)entry->path); } - oidmap_clear(&missing_objects, true); + oidmap_clear_with_free(&missing_objects, free_missing_objects_entry); } stop_progress(&progress); diff --git a/list-objects-filter.c b/list-objects-filter.c index 78316e7f90..0038bfaac5 100644 --- a/list-objects-filter.c +++ b/list-objects-filter.c @@ -143,6 +143,13 @@ struct seen_map_entry { size_t depth; }; +static void free_seen_map_entry(void *e) +{ + struct seen_map_entry *entry = + container_of(e, struct seen_map_entry, base); + free(entry); +} + /* Returns 1 if the oid was in the omits set before it was invoked. */ static int filter_trees_update_omits( struct object *obj, @@ -244,7 +251,7 @@ static void filter_trees_free(void *filter_data) { struct filter_trees_depth_data *d = filter_data; if (!d) return; - oidmap_clear(&d->seen_at_depth, 1); + oidmap_clear_with_free(&d->seen_at_depth, free_seen_map_entry); free(d); } diff --git a/odb.c b/odb.c index 84a31084d3..21ea1cb799 100644 --- a/odb.c +++ b/odb.c @@ -13,6 +13,7 @@ #include "object-file-convert.h" #include "object-file.h" #include "odb.h" +#include "oidmap.h" #include "packfile.h" #include "path.h" #include "promisor-remote.h" @@ -977,6 +978,13 @@ void odb_close(struct object_database *o) close_commit_graph(o); } +static void free_replace_map_entry(void *e) +{ + struct replace_object *entry = + container_of(e, struct replace_object, original); + free(entry); +} + static void odb_free_sources(struct object_database *o) { while (o->sources) { @@ -997,7 +1005,8 @@ void odb_free(struct object_database *o) free(o->alternate_db); - oidmap_clear(&o->replace_map, 1); + if (o->replace_map_initialized) + oidmap_clear_with_free(&o->replace_map, free_replace_map_entry); pthread_mutex_destroy(&o->replace_mutex); odb_close(o); diff --git a/oidmap.c b/oidmap.c index 508d6c7dec..a1ef53577f 100644 --- a/oidmap.c +++ b/oidmap.c @@ -24,11 +24,28 @@ void oidmap_init(struct oidmap *map, size_t initial_size) void oidmap_clear(struct oidmap *map, int free_entries) { - if (!map) + oidmap_clear_with_free(map, + free_entries ? free : NULL); +} + +void oidmap_clear_with_free(struct oidmap *map, + oidmap_free_fn free_fn) +{ + struct hashmap_iter iter; + struct hashmap_entry *e; + + if (!map || !map->map.cmpfn) return; - /* TODO: make oidmap itself not depend on struct layouts */ - hashmap_clear_(&map->map, free_entries ? 0 : -1); + hashmap_iter_init(&map->map, &iter); + while ((e = hashmap_iter_next(&iter))) { + struct oidmap_entry *entry = + container_of(e, struct oidmap_entry, internal_entry); + if (free_fn) + free_fn(entry); + } + + hashmap_clear(&map->map); } void *oidmap_get(const struct oidmap *map, const struct object_id *key) diff --git a/oidmap.h b/oidmap.h index 67fb32290f..acddcaecdd 100644 --- a/oidmap.h +++ b/oidmap.h @@ -35,6 +35,21 @@ struct oidmap { */ void oidmap_init(struct oidmap *map, size_t initial_size); +/* + * Function type for functions that free oidmap entries. + */ +typedef void (*oidmap_free_fn)(void *); + +/* + * Clear an oidmap, freeing any allocated memory. The map is empty and + * can be reused without another explicit init. + * + * The `free_fn`, if not NULL, is called for each oidmap entry in the map + * to free any user data associated with the entry. + */ +void oidmap_clear_with_free(struct oidmap *map, + oidmap_free_fn free_fn); + /* * Clear an oidmap, freeing any allocated memory. The map is empty and * can be reused without another explicit init. diff --git a/sequencer.c b/sequencer.c index f794770bac..5808a293ad 100644 --- a/sequencer.c +++ b/sequencer.c @@ -5705,6 +5705,12 @@ struct string_entry { char string[FLEX_ARRAY]; }; +static void free_string_entry(void *e) +{ + struct string_entry *entry = container_of(e, struct string_entry, entry); + free(entry); +} + struct label_state { struct oidmap commit2label; struct hashmap labels; @@ -6095,8 +6101,8 @@ static int make_script_with_merges(struct pretty_print_context *pp, oidset_clear(&interesting); oidset_clear(&child_seen); oidset_clear(&shown); - oidmap_clear(&commit2todo, 1); - oidmap_clear(&state.commit2label, 1); + oidmap_clear_with_free(&commit2todo, free_string_entry); + oidmap_clear_with_free(&state.commit2label, free_string_entry); hashmap_clear_and_free(&state.labels, struct labels_entry, entry); strbuf_release(&state.buf); diff --git a/t/unit-tests/u-oidmap.c b/t/unit-tests/u-oidmap.c index b23af449f6..00481df63f 100644 --- a/t/unit-tests/u-oidmap.c +++ b/t/unit-tests/u-oidmap.c @@ -14,6 +14,13 @@ struct test_entry { char name[FLEX_ARRAY]; }; +static int freed; + +static void test_free_fn(void *p) { + freed++; + free(p); +} + static const char *const key_val[][2] = { { "11", "one" }, { "22", "two" }, { "33", "three" } }; @@ -134,3 +141,37 @@ void test_oidmap__iterate(void) cl_assert_equal_i(count, ARRAY_SIZE(key_val)); cl_assert_equal_i(hashmap_get_size(&map.map), ARRAY_SIZE(key_val)); } + +void test_oidmap__clear_without_free_callback(void) +{ + struct oidmap local_map = OIDMAP_INIT; + struct test_entry *entry; + + freed = 0; + + FLEX_ALLOC_STR(entry, name, "one"); + cl_parse_any_oid("11", &entry->entry.oid); + cl_assert(oidmap_put(&local_map, entry) == NULL); + + oidmap_clear_with_free(&local_map, NULL); + + cl_assert_equal_i(freed, 0); + + free(entry); +} + +void test_oidmap__clear_with_free_callback(void) +{ + struct oidmap local_map = OIDMAP_INIT; + struct test_entry *entry; + + freed = 0; + + FLEX_ALLOC_STR(entry, name, "one"); + cl_parse_any_oid("11", &entry->entry.oid); + cl_assert(oidmap_put(&local_map, entry) == NULL); + + oidmap_clear_with_free(&local_map, test_free_fn); + + cl_assert_equal_i(freed, 1); +}