{"thread":{"id":"64812","subject":"[PATCH 0/1] oidmap: make entry cleanup explicit in oidmap_clear","startedAt":"2026-01-15T13:17:05Z","lastAt":"2026-01-15T13:17:10Z","messageCount":2,"participants":["Seyi Kufoiji"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"533953","messageId":"20260115131634.51968-1-kuforiji98@gmail.com","threadId":"64812","inReplyTo":null,"subject":"[PATCH 0/1] oidmap: make entry cleanup explicit in oidmap_clear","fromName":"Seyi Kufoiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-01-15T13:16:33Z","receivedAt":"2026-01-15T13:17:05Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Hello,\n\nThis small patch improves the clarity and safety of oidmap cleanup by\nmaking entry destruction explicit in oidmap_clear().\n\nPreviously, oidmap_clear() implicitly relied on callers to understand\nhow and when stored entries were freed. This change introduces an\nexplicit cleanup path, making ownership and lifecycle of oidmap entries\nclearer and less error-prone.\n\nIn addition to the implementation change, unit tests are added to\nexercise the cleanup semantics and verify correct behavior during map\nteardown.\n\nThanks,\nSeyi\n\nSeyi Kufoiji (1):\n oidmap: make entry cleanup explicit in oidmap_clear\n\n oidmap.c | 23 ++++++++++++++++++++---\n oidmap.h | 15 +++++++++++++++\n t/unit-tests/u-oidmap.c | 41 +++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 76 insertions(+), 3 deletions(-)\n\n-- \n2.43.0\n\n"},{"id":"533954","messageId":"20260115131634.51968-2-kuforiji98@gmail.com","threadId":"64812","inReplyTo":"20260115131634.51968-1-kuforiji98@gmail.com","subject":"[PATCH 1/1] oidmap: make entry cleanup explicit in oidmap_clear","fromName":"Seyi Kufoiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-01-15T13:16:34Z","receivedAt":"2026-01-15T13:17:10Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Replace oidmap's use of hashmap_clear_() and layout-dependent freeing\nwith an explicit iteration and optional free callback. This removes\nreliance on struct layout assumptions while keeping the existing API\nintact.\n\nAdd tests for oidmap_clear_with_free behavior.\ntest_oidmap__clear_with_free_callback verifies that entries are freed\nwhen a callback is provided, while\ntest_oidmap__clear_without_free_callback verifies that entries are not\nfreed when no callback is given. These tests ensure the new clear\nimplementation behaves correctly and preserves ownership semantics.\n\nSigned-off-by: Seyi Kuforiji \n---\n oidmap.c | 23 ++++++++++++++++++++---\n oidmap.h | 15 +++++++++++++++\n t/unit-tests/u-oidmap.c | 41 +++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 76 insertions(+), 3 deletions(-)\n\ndiff --git a/oidmap.c b/oidmap.c\nindex 508d6c7dec..a1ef53577f 100644\n--- a/oidmap.c\n+++ b/oidmap.c\n@@ -24,11 +24,28 @@ void oidmap_init(struct oidmap *map, size_t initial_size)\n \n void oidmap_clear(struct oidmap *map, int free_entries)\n {\n-\tif (!map)\n+\toidmap_clear_with_free(map,\n+\t\tfree_entries ? free : NULL);\n+}\n+\n+void oidmap_clear_with_free(struct oidmap *map,\n+\t\t\t oidmap_free_fn free_fn)\n+{\n+\tstruct hashmap_iter iter;\n+\tstruct hashmap_entry *e;\n+\n+\tif (!map || !map->map.cmpfn)\n \t\treturn;\n \n-\t/* TODO: make oidmap itself not depend on struct layouts */\n-\thashmap_clear_(&map->map, free_entries ? 0 : -1);\n+\thashmap_iter_init(&map->map, &iter);\n+\twhile ((e = hashmap_iter_next(&iter))) {\n+\t\tstruct oidmap_entry *entry =\n+\t\t\tcontainer_of(e, struct oidmap_entry, internal_entry);\n+\t\tif (free_fn)\n+\t\t\tfree_fn(entry);\n+\t}\n+\n+\thashmap_clear(&map->map);\n }\n \n void *oidmap_get(const struct oidmap *map, const struct object_id *key)\ndiff --git a/oidmap.h b/oidmap.h\nindex 67fb32290f..acddcaecdd 100644\n--- a/oidmap.h\n+++ b/oidmap.h\n@@ -35,6 +35,21 @@ struct oidmap {\n */\n void oidmap_init(struct oidmap *map, size_t initial_size);\n \n+/*\n+ * Function type for functions that free oidmap entries.\n+ */\n+typedef void (*oidmap_free_fn)(void *);\n+\n+/*\n+ * Clear an oidmap, freeing any allocated memory. The map is empty and\n+ * can be reused without another explicit init.\n+ *\n+ * The `free_fn`, if not NULL, is called for each oidmap entry in the map\n+ * to free any user data associated with the entry.\n+ */\n+void oidmap_clear_with_free(struct oidmap *map,\n+\t\t\t oidmap_free_fn free_fn);\n+\n /*\n * Clear an oidmap, freeing any allocated memory. The map is empty and\n * can be reused without another explicit init.\ndiff --git a/t/unit-tests/u-oidmap.c b/t/unit-tests/u-oidmap.c\nindex b23af449f6..00481df63f 100644\n--- a/t/unit-tests/u-oidmap.c\n+++ b/t/unit-tests/u-oidmap.c\n@@ -14,6 +14,13 @@ struct test_entry {\n \tchar name[FLEX_ARRAY];\n };\n \n+static int freed;\n+\n+static void test_free_fn(void *p) {\n+\tfreed++;\n+\tfree(p);\n+}\n+\n static const char *const key_val[][2] = { { \"11\", \"one\" },\n \t\t\t\t\t { \"22\", \"two\" },\n \t\t\t\t\t { \"33\", \"three\" } };\n@@ -134,3 +141,37 @@ void test_oidmap__iterate(void)\n \tcl_assert_equal_i(count, ARRAY_SIZE(key_val));\n \tcl_assert_equal_i(hashmap_get_size(&map.map), ARRAY_SIZE(key_val));\n }\n+\n+void test_oidmap__clear_without_free_callback(void)\n+{\n+\tstruct oidmap local_map = OIDMAP_INIT;\n+\tstruct test_entry *entry;\n+\n+\tfreed = 0;\n+\n+\tFLEX_ALLOC_STR(entry, name, \"one\");\n+\tcl_parse_any_oid(\"11\", &entry->entry.oid);\n+\tcl_assert(oidmap_put(&local_map, entry) == NULL);\n+\n+\toidmap_clear_with_free(&local_map, NULL);\n+\n+\tcl_assert_equal_i(freed, 0);\n+\n+\tfree(entry);\n+}\n+\n+void test_oidmap__clear_with_free_callback(void)\n+{\n+\tstruct oidmap local_map = OIDMAP_INIT;\n+\tstruct test_entry *entry;\n+\n+\tfreed = 0;\n+\n+\tFLEX_ALLOC_STR(entry, name, \"one\");\n+\tcl_parse_any_oid(\"11\", &entry->entry.oid);\n+\tcl_assert(oidmap_put(&local_map, entry) == NULL);\n+\n+\toidmap_clear_with_free(&local_map, test_free_fn);\n+\n+\tcl_assert_equal_i(freed, 1);\n+}\n-- \n2.43.0\n\n"}]}