Skip to content

Commit 422aaa6

Browse files
committed
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>
1 parent 8745eae commit 422aaa6

File tree

3 files changed

+76
-3
lines changed

3 files changed

+76
-3
lines changed

oidmap.c

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,28 @@ void oidmap_init(struct oidmap *map, size_t initial_size)
2424

2525
void oidmap_clear(struct oidmap *map, int free_entries)
2626
{
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)
2838
return;
2939

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);
3249
}
3350

3451
void *oidmap_get(const struct oidmap *map, const struct object_id *key)

oidmap.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,21 @@ struct oidmap {
3535
*/
3636
void oidmap_init(struct oidmap *map, size_t initial_size);
3737

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+
3853
/*
3954
* Clear an oidmap, freeing any allocated memory. The map is empty and
4055
* can be reused without another explicit init.

t/unit-tests/u-oidmap.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ struct test_entry {
1414
char name[FLEX_ARRAY];
1515
};
1616

17+
static int freed;
18+
19+
static void test_free_fn(void *p) {
20+
freed++;
21+
free(p);
22+
}
23+
1724
static const char *const key_val[][2] = { { "11", "one" },
1825
{ "22", "two" },
1926
{ "33", "three" } };
@@ -134,3 +141,37 @@ void test_oidmap__iterate(void)
134141
cl_assert_equal_i(count, ARRAY_SIZE(key_val));
135142
cl_assert_equal_i(hashmap_get_size(&map.map), ARRAY_SIZE(key_val));
136143
}
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+
}

0 commit comments

Comments
 (0)