oidmap: make entry cleanup explicit in oidmap_clear

Replace oidmap's use of hashmap_clear_() and layout-dependent freeing with an explicit iteration and optional free callback. This removes reliance on struct layout assumptions while keeping the existing API intact. Add tests for oidmap_clear_with_free behavior. test_oidmap__clear_with_free_callback verifies that entries are freed when a callback is provided, while test_oidmap__clear_without_free_callback verifies that entries are not freed when no callback is given. These tests ensure the new clear implementation behaves correctly and preserves ownership semantics. Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Seyi Kufoiji committed Mar 5, 2026 at 11:05 UTC a06a725c7847840ac56c0e797a829ac13abbe350
3 files changed +76 -3
oidmap.c
+20 -3
@@ -24,11 +24,28 @@ void oidmap_init(struct oidmap *map, size_t initial_size)
24
25 void oidmap_clear(struct oidmap *map, int free_entries)
26 {
27 - if (!map)
27 + oidmap_clear_with_free(map,
28 + free_entries ? free : NULL);
29 +}
30 +
31 +void oidmap_clear_with_free(struct oidmap *map,
32 + oidmap_free_fn free_fn)
33 +{
34 + struct hashmap_iter iter;
35 + struct hashmap_entry *e;
36 +
37 + if (!map || !map->map.cmpfn)
38 return;
39
30 - /* TODO: make oidmap itself not depend on struct layouts */
31 - hashmap_clear_(&map->map, free_entries ? 0 : -1);
40 + hashmap_iter_init(&map->map, &iter);
41 + while ((e = hashmap_iter_next(&iter))) {
42 + struct oidmap_entry *entry =
43 + container_of(e, struct oidmap_entry, internal_entry);
44 + if (free_fn)
45 + free_fn(entry);
46 + }
47 +
48 + hashmap_clear(&map->map);
49 }
50
51 void *oidmap_get(const struct oidmap *map, const struct object_id *key)
oidmap.h
+15
@@ -35,6 +35,21 @@ struct oidmap {
35 */
36 void oidmap_init(struct oidmap *map, size_t initial_size);
37
38 +/*
39 + * Function type for functions that free oidmap entries.
40 + */
41 +typedef void (*oidmap_free_fn)(void *);
42 +
43 +/*
44 + * Clear an oidmap, freeing any allocated memory. The map is empty and
45 + * can be reused without another explicit init.
46 + *
47 + * The `free_fn`, if not NULL, is called for each oidmap entry in the map
48 + * to free any user data associated with the entry.
49 + */
50 +void oidmap_clear_with_free(struct oidmap *map,
51 + oidmap_free_fn free_fn);
52 +
53 /*
54 * Clear an oidmap, freeing any allocated memory. The map is empty and
55 * can be reused without another explicit init.
t/unit-tests/u-oidmap.c
+41
@@ -14,6 +14,13 @@ struct test_entry {
14 char name[FLEX_ARRAY];
15 };
16
17 +static int freed;
18 +
19 +static void test_free_fn(void *p) {
20 + freed++;
21 + free(p);
22 +}
23 +
24 static const char *const key_val[][2] = { { "11", "one" },
25 { "22", "two" },
26 { "33", "three" } };
@@ -134,3 +141,37 @@ void test_oidmap__iterate(void)
141 cl_assert_equal_i(count, ARRAY_SIZE(key_val));
142 cl_assert_equal_i(hashmap_get_size(&map.map), ARRAY_SIZE(key_val));
143 }
144 +
145 +void test_oidmap__clear_without_free_callback(void)
146 +{
147 + struct oidmap local_map = OIDMAP_INIT;
148 + struct test_entry *entry;
149 +
150 + freed = 0;
151 +
152 + FLEX_ALLOC_STR(entry, name, "one");
153 + cl_parse_any_oid("11", &entry->entry.oid);
154 + cl_assert(oidmap_put(&local_map, entry) == NULL);
155 +
156 + oidmap_clear_with_free(&local_map, NULL);
157 +
158 + cl_assert_equal_i(freed, 0);
159 +
160 + free(entry);
161 +}
162 +
163 +void test_oidmap__clear_with_free_callback(void)
164 +{
165 + struct oidmap local_map = OIDMAP_INIT;
166 + struct test_entry *entry;
167 +
168 + freed = 0;
169 +
170 + FLEX_ALLOC_STR(entry, name, "one");
171 + cl_parse_any_oid("11", &entry->entry.oid);
172 + cl_assert(oidmap_put(&local_map, entry) == NULL);
173 +
174 + oidmap_clear_with_free(&local_map, test_free_fn);
175 +
176 + cl_assert_equal_i(freed, 1);
177 +}