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>

diff --git a/oidmap.c b/oidmap.c
index 508d6c7dec17a71e16afc63e7050babcdc7c03a6..a1ef53577fbb405745351639c78ee7b8ae4f161e 100644 (file)
--- 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 67fb32290f1c494a36be653a908f37b58f5e6202..acddcaecdd7fcab7163dda84dc703abe6583d2f6 100644 (file)
--- 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/t/unit-tests/u-oidmap.c b/t/unit-tests/u-oidmap.c
index b23af449f6452e5733329c8d3a1274563f42ceea..00481df63f52cca93c6f070898aac4922614a731 100644 (file)
--- 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);
+}