Skip to content

Commit 9a591a9

Browse files
committed
Merge branch 'sk/oidmap-clear-with-custom-free-func'
A bit of OIDmap API enhancement and cleanup. * sk/oidmap-clear-with-custom-free-func: builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free() oidmap: make entry cleanup explicit in oidmap_clear
2 parents 642aa4f + a98ea50 commit 9a591a9

4 files changed

Lines changed: 88 additions & 6 deletions

File tree

builtin/rev-list.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,19 @@ static int arg_print_omitted; /* print objects omitted by filter */
8888

8989
struct missing_objects_map_entry {
9090
struct oidmap_entry entry;
91-
const char *path;
91+
char *path;
9292
unsigned type;
9393
};
94+
95+
static void missing_objects_map_entry_free(void *e)
96+
{
97+
struct missing_objects_map_entry *entry =
98+
container_of(e, struct missing_objects_map_entry, entry);
99+
100+
free(entry->path);
101+
free(entry);
102+
}
103+
94104
static struct oidmap missing_objects;
95105
enum missing_action {
96106
MA_ERROR = 0, /* fail if any missing objects are encountered */
@@ -935,10 +945,9 @@ int cmd_rev_list(int argc,
935945
while ((entry = oidmap_iter_next(&iter))) {
936946
print_missing_object(entry, arg_missing_action ==
937947
MA_PRINT_INFO);
938-
free((void *)entry->path);
939948
}
940949

941-
oidmap_clear(&missing_objects, true);
950+
oidmap_clear_with_free(&missing_objects, missing_objects_map_entry_free);
942951
}
943952

944953
stop_progress(&progress);

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)