{"thread":{"id":"65093","subject":"[PATCH 0/5] oidmap: migrate cleanup to oidmap_clear_with_free()","startedAt":"2026-02-27T23:42:44Z","lastAt":"2026-03-05T19:17:44Z","messageCount":30,"participants":["Seyi Kuforiji","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"537360","messageId":"20260227234213.17633-1-kuforiji98@gmail.com","threadId":"65093","inReplyTo":null,"subject":"[PATCH 0/5] oidmap: migrate cleanup to oidmap_clear_with_free()","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-02-27T23:42:08Z","receivedAt":"2026-02-27T23:42:44Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Hi,\n\nThis series replaces oidmap_clear(map, 1) with\noidmap_clear_with_free() and introduces explicit free callbacks\nat the remaining call sites.\n\nThe old boolean-based API implicitly assumed plain free(),\nwhich obscures ownership semantics and does not work well\nwhen oidmap_entry is embedded inside larger structures.\nThe callback-based API makes cleanup explicit and type-safe,\nand avoids relying on hidden assumptions about allocation.\n\nThis improves readability, maintainability, and correctness,\nand makes future refactoring of oidmap users more robust.\n\nThis is used in subsequent commits to adequately cleanup all\nusage site.\n\nThanks,\nSeyi Kuforiji\n\nSeyi Kufoiji (5):\n oidmap: make entry cleanup explicit in oidmap_clear\n builtin/rev-list: migrate missing_objects cleanup to\n oidmap_clear_with_free()\n list-objects-filter: use oidmap_clear_with_free() for cleanup\n odb: use oidmap_clear_with_free() to release replace_map entries\n sequencer: use oidmap_clear_with_free() for string_entry cleanup\n\n builtin/rev-list.c | 13 ++++++++++---\n list-objects-filter.c | 9 ++++++++-\n odb.c | 11 ++++++++++-\n oidmap.c | 23 ++++++++++++++++++++---\n oidmap.h | 15 +++++++++++++++\n sequencer.c | 10 ++++++++--\n t/unit-tests/u-oidmap.c | 41 +++++++++++++++++++++++++++++++++++++++++\n 7 files changed, 112 insertions(+), 10 deletions(-)\n\n-- \n2.43.0\n\n"},{"id":"537361","messageId":"20260227234213.17633-2-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260227234213.17633-1-kuforiji98@gmail.com","subject":"[PATCH 1/5] oidmap: make entry cleanup explicit in oidmap_clear","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-02-27T23:42:09Z","receivedAt":"2026-02-27T23:42:50Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nReplace 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"},{"id":"537362","messageId":"20260227234213.17633-3-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260227234213.17633-1-kuforiji98@gmail.com","subject":"[PATCH 2/5] builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free()","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-02-27T23:42:10Z","receivedAt":"2026-02-27T23:42:56Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nAs part of the conversion away from oidmap_clear(), switch the\nmissing_objects map to use oidmap_clear_with_free().\n\nmissing_objects stores struct missing_objects_map_entry instances,\nwhich own an xstrdup()'d path string in addition to the container\nstruct itself. Previously, rev-list manually freed entry->path\nbefore calling oidmap_clear(&missing_objects, true).\n\nIntroduce a dedicated free callback and pass it to\noidmap_clear_with_free(), consolidating entry teardown into a\nsingle place and making cleanup semantics explicit. This improves\nclarity and maintainability.\n\nSigned-off-by: Seyi Kuforiji \n---\n builtin/rev-list.c | 13 ++++++++++---\n 1 file changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex ddea8aa251..567dc5e7f5 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -88,9 +88,17 @@ static int arg_print_omitted; /* print objects omitted by filter */\n \n struct missing_objects_map_entry {\n \tstruct oidmap_entry entry;\n-\tconst char *path;\n+\tchar *path;\n \tunsigned type;\n };\n+\n+static void free_missing_objects_entry(void *e)\n+{\n+\tstruct missing_objects_map_entry *entry =\n+\t\tcontainer_of(e, struct missing_objects_map_entry, entry);\n+\tfree(entry);\n+}\n+\n static struct oidmap missing_objects;\n enum missing_action {\n \tMA_ERROR = 0, /* fail if any missing objects are encountered */\n@@ -935,10 +943,9 @@ int cmd_rev_list(int argc,\n \t\twhile ((entry = oidmap_iter_next(&iter))) {\n \t\t\tprint_missing_object(entry, arg_missing_action ==\n \t\t\t\t\t\t\t MA_PRINT_INFO);\n-\t\t\tfree((void *)entry->path);\n \t\t}\n \n-\t\toidmap_clear(&missing_objects, true);\n+\t\toidmap_clear_with_free(&missing_objects, free_missing_objects_entry);\n \t}\n \n \tstop_progress(&progress);\n-- \n2.43.0\n\n"},{"id":"537363","messageId":"20260227234213.17633-4-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260227234213.17633-1-kuforiji98@gmail.com","subject":"[PATCH 3/5] list-objects-filter: use oidmap_clear_with_free() for cleanup","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-02-27T23:42:11Z","receivedAt":"2026-02-27T23:43:01Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nReplace the use of oidmap_clear(&seen_at_depth, 1) in\nfilter_trees_free() with oidmap_clear_with_free().\n\nThe seen_at_depth map stores heap-allocated struct\nseen_map_entry objects. Previously, passing 1 relied on\noidmap_clear() internally calling free() on each entry.\n\nConvert this to the explicit oidmap_clear_with_free() API\nand provide a typed free_seen_map_entry() helper to free\neach container entry.\n\nThis makes the ownership and cleanup policy explicit and\nremoves reliance on the legacy boolean free_entries\nparameter.\n\nSigned-off-by: Seyi Kuforiji \n---\n list-objects-filter.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/list-objects-filter.c b/list-objects-filter.c\nindex 78316e7f90..0038bfaac5 100644\n--- a/list-objects-filter.c\n+++ b/list-objects-filter.c\n@@ -143,6 +143,13 @@ struct seen_map_entry {\n \tsize_t depth;\n };\n \n+static void free_seen_map_entry(void *e)\n+{\n+\tstruct seen_map_entry *entry =\n+\t\tcontainer_of(e, struct seen_map_entry, base);\n+\tfree(entry);\n+}\n+\n /* Returns 1 if the oid was in the omits set before it was invoked. */\n static int filter_trees_update_omits(\n \tstruct object *obj,\n@@ -244,7 +251,7 @@ static void filter_trees_free(void *filter_data) {\n \tstruct filter_trees_depth_data *d = filter_data;\n \tif (!d)\n \t\treturn;\n-\toidmap_clear(&d->seen_at_depth, 1);\n+\toidmap_clear_with_free(&d->seen_at_depth, free_seen_map_entry);\n \tfree(d);\n }\n \n-- \n2.43.0\n\n"},{"id":"537364","messageId":"20260227234213.17633-5-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260227234213.17633-1-kuforiji98@gmail.com","subject":"[PATCH 4/5] odb: use oidmap_clear_with_free() to release replace_map entries","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-02-27T23:42:12Z","receivedAt":"2026-02-27T23:43:07Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nReplace the direct oidmap_clear() call in odb_free() with\noidmap_clear_with_free(), and introduce a free_replace_map_entry()\nhelper to properly free each struct replace_object stored in the map.\n\nThis centralizes cleanup logic and ensures entries are released\ncorrectly via a dedicated callback.\n\nSigned-off-by: Seyi Kuforiji \n---\n odb.c | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/odb.c b/odb.c\nindex 1679cc0465..8ca497203f 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -14,6 +14,7 @@\n #include \"object-file-convert.h\"\n #include \"object-file.h\"\n #include \"odb.h\"\n+#include \"oidmap.h\"\n #include \"packfile.h\"\n #include \"path.h\"\n #include \"promisor-remote.h\"\n@@ -1089,6 +1090,13 @@ void odb_close(struct object_database *o)\n \tclose_commit_graph(o);\n }\n \n+static void free_replace_map_entry(void *e)\n+{\n+\tstruct replace_object *entry =\n+\t\tcontainer_of(e, struct replace_object, original);\n+\tfree(entry);\n+}\n+\n static void odb_free_sources(struct object_database *o)\n {\n \twhile (o->sources) {\n@@ -1109,7 +1117,8 @@ void odb_free(struct object_database *o)\n \n \tfree(o->alternate_db);\n \n-\toidmap_clear(&o->replace_map, 1);\n+\tif (o->replace_map_initialized)\n+\t\toidmap_clear_with_free(&o->replace_map, free_replace_map_entry);\n \tpthread_mutex_destroy(&o->replace_mutex);\n \n \todb_close(o);\n-- \n2.43.0\n\n"},{"id":"537365","messageId":"20260227234213.17633-6-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260227234213.17633-1-kuforiji98@gmail.com","subject":"[PATCH 5/5] sequencer: use oidmap_clear_with_free() for string_entry cleanup","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-02-27T23:42:13Z","receivedAt":"2026-02-27T23:43:12Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nSwitch cleanup of the string_entry oidmap to\noidmap_clear_with_free() and introduce a free_string_entry()\nhelper to properly free each allocated struct string_entry.\n\nThis aligns with the ongoing migration to use the callback-based\noidmap cleanup API.\n\nSigned-off-by: Seyi Kuforiji \n---\n sequencer.c | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex a3eb39bb25..75ef2ace4f 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -5654,6 +5654,12 @@ struct string_entry {\n \tchar string[FLEX_ARRAY];\n };\n \n+static void free_string_entry(void *e)\n+{\n+\tstruct string_entry *entry = container_of(e, struct string_entry, entry);\n+\tfree(entry);\n+}\n+\n struct label_state {\n \tstruct oidmap commit2label;\n \tstruct hashmap labels;\n@@ -6044,8 +6050,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \toidset_clear(&interesting);\n \toidset_clear(&child_seen);\n \toidset_clear(&shown);\n-\toidmap_clear(&commit2todo, 1);\n-\toidmap_clear(&state.commit2label, 1);\n+\toidmap_clear_with_free(&commit2todo, free_string_entry);\n+\toidmap_clear_with_free(&state.commit2label, free_string_entry);\n \thashmap_clear_and_free(&state.labels, struct labels_entry, entry);\n \tstrbuf_release(&state.buf);\n \n-- \n2.43.0\n\n"},{"id":"537366","messageId":"xmqqcy1pohjz.fsf@gitster.g","threadId":"65093","inReplyTo":"20260227234213.17633-3-kuforiji98@gmail.com","subject":"Re: [PATCH 2/5] builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-28T00:09:04Z","receivedAt":"2026-02-28T00:09:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seyi Kuforiji writes:\n\n> From: Seyi Kufoiji \n>\n> As part of the conversion away from oidmap_clear(), switch the\n> missing_objects map to use oidmap_clear_with_free().\n>\n> missing_objects stores struct missing_objects_map_entry instances,\n> which own an xstrdup()'d path string in addition to the container\n> struct itself. Previously, rev-list manually freed entry->path\n> before calling oidmap_clear(&missing_objects, true).\n>\n> Introduce a dedicated free callback and pass it to\n> oidmap_clear_with_free(), consolidating entry teardown into a\n> single place and making cleanup semantics explicit. This improves\n> clarity and maintainability.\n>\n> Signed-off-by: Seyi Kuforiji \n> ---\n> builtin/rev-list.c | 13 ++++++++++---\n> 1 file changed, 10 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> index ddea8aa251..567dc5e7f5 100644\n> --- a/builtin/rev-list.c\n> +++ b/builtin/rev-list.c\n> @@ -88,9 +88,17 @@ static int arg_print_omitted; /* print objects omitted by filter */\n> \n> struct missing_objects_map_entry {\n> \tstruct oidmap_entry entry;\n> -\tconst char *path;\n> +\tchar *path;\n> \tunsigned type;\n> };\n> +\n> +static void free_missing_objects_entry(void *e)\n> +{\n> +\tstruct missing_objects_map_entry *entry =\n> +\t\tcontainer_of(e, struct missing_objects_map_entry, entry);\n> +\tfree(entry);\n> +}\n> +\n> static struct oidmap missing_objects;\n> enum missing_action {\n> \tMA_ERROR = 0, /* fail if any missing objects are encountered */\n> @@ -935,10 +943,9 @@ int cmd_rev_list(int argc,\n> \t\twhile ((entry = oidmap_iter_next(&iter))) {\n> \t\t\tprint_missing_object(entry, arg_missing_action ==\n> \t\t\t\t\t\t\t MA_PRINT_INFO);\n> -\t\t\tfree((void *)entry->path);\n> \t\t}\n> \n> -\t\toidmap_clear(&missing_objects, true);\n> +\t\toidmap_clear_with_free(&missing_objects, free_missing_objects_entry);\n> \t}\n\nHmph, maybe I am confused, but the shape and memory ownership of the\nstructure involved has not changed before or after this patch. We\nhave bunch of missing_objects_map_entry instances that embeds\noidmap_entry in each of them, and we iterate over this oidmap,\nenumerating all the missing_objects_map_entry instances.\n\nIn the original code, we used to not just clear the oidmap used to\nhold these missing_objects_map_entry instances, but dropped the\nstring pointed at and owned by the .path member of them while\niterating. With the new code, I can see the newer interface\noidmap_clear_with_free() being told how to reclaim resources held by\nthese missing_objects_map_entry instances, so it is a perfect place\nto not just free the \"shell\" structure, but also the piece of memory\nheld by the .path member, isn't it? It appears to me that the\nstring pointed at by .path is no longer freed, introducing a leak?\n\nThe above hardly convinces me of the claim in the proposed log\nmessage that this change improves clarity nor maintainability. I am\npuzzled...\n"},{"id":"537596","messageId":"20260302200018.75731-1-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260227234213.17633-1-kuforiji98@gmail.com","subject":"[PATCH v2 0/5] oidmap: migrate cleanup to oidmap_clear_with_free()","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-03-02T20:00:12Z","receivedAt":"2026-03-02T20:00:57Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Hi,\n\nThis series replaces oidmap_clear(map, 1) with\noidmap_clear_with_free() and introduces explicit free callbacks\nat the remaining call sites.\n\nThe old boolean-based API implicitly assumed plain free(),\nwhich obscures ownership semantics and does not work well\nwhen oidmap_entry is embedded inside larger structures.\nThe callback-based API makes cleanup explicit and type-safe,\nand avoids relying on hidden assumptions about allocation.\n\nThis is used in subsequent commits to adequately cleanup all\nusage site.\n\nChanges in v2:\n - fix missing .path cleanup\n - modified commit message to be more accurate\n\nThanks\nSeyi\n\nSeyi Kufoiji (5):\n oidmap: make entry cleanup explicit in oidmap_clear\n builtin/rev-list: migrate missing_objects cleanup to\n oidmap_clear_with_free()\n list-objects-filter: use oidmap_clear_with_free() for cleanup\n odb: use oidmap_clear_with_free() to release replace_map entries\n sequencer: use oidmap_clear_with_free() for string_entry cleanup\n\n builtin/rev-list.c | 15 ++++++++++++---\n list-objects-filter.c | 9 ++++++++-\n odb.c | 11 ++++++++++-\n oidmap.c | 23 ++++++++++++++++++++---\n oidmap.h | 15 +++++++++++++++\n sequencer.c | 10 ++++++++--\n t/unit-tests/u-oidmap.c | 41 +++++++++++++++++++++++++++++++++++++++++\n 7 files changed, 114 insertions(+), 10 deletions(-)\n\nRange-diff against v1:\n1: a0ab068630 < -: ---------- sparse-checkout: use string_list_sort_u\n2: b2c0ee2593 < -: ---------- The 7th batch\n3: 6af69c2c49 = 1: 1d544ef7d2 oidmap: make entry cleanup explicit in oidmap_clear\n4: 452f5d8edb ! 2: f2c3a699bd builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free()\n @@ Metadata\n Author: Seyi Kufoiji \n \n ## Commit message ##\n - builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free()\n + builtin/rev-list: migrate missing_objects cleanup to\n + oidmap_clear_with_free()\n \n As part of the conversion away from oidmap_clear(), switch the\n missing_objects map to use oidmap_clear_with_free().\n @@ Commit message\n \n Introduce a dedicated free callback and pass it to\n oidmap_clear_with_free(), consolidating entry teardown into a\n - single place and making cleanup semantics explicit. This improves\n - clarity and maintainability.\n + single place and making cleanup semantics explicit.\n \n Signed-off-by: Seyi Kuforiji \n \n @@ builtin/rev-list.c: static int arg_print_omitted; /* print objects omitted by fi\n +{\n +\tstruct missing_objects_map_entry *entry =\n +\t\tcontainer_of(e, struct missing_objects_map_entry, entry);\n ++\n ++\tfree(entry->path);\n +\tfree(entry);\n +}\n +\n5: cbfcc4e9cc = 3: a4e426bcca list-objects-filter: use oidmap_clear_with_free() for cleanup\n6: bfbdb51d84 = 4: 4116e5491d odb: use oidmap_clear_with_free() to release replace_map entries\n7: 4fc6e71f4c = 5: ad1f776a19 sequencer: use oidmap_clear_with_free() for string_entry cleanup\n-- \n2.43.0\n\n"},{"id":"537597","messageId":"20260302200018.75731-2-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260302200018.75731-1-kuforiji98@gmail.com","subject":"[PATCH v2 1/5] oidmap: make entry cleanup explicit in oidmap_clear","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-03-02T20:00:13Z","receivedAt":"2026-03-02T20:01:05Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nReplace 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"},{"id":"537598","messageId":"20260302200018.75731-3-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260302200018.75731-1-kuforiji98@gmail.com","subject":"[PATCH v2 2/5] builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free()","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-03-02T20:00:14Z","receivedAt":"2026-03-02T20:01:13Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nAs part of the conversion away from oidmap_clear(), switch the\nmissing_objects map to use oidmap_clear_with_free().\n\nmissing_objects stores struct missing_objects_map_entry instances,\nwhich own an xstrdup()'d path string in addition to the container\nstruct itself. Previously, rev-list manually freed entry->path\nbefore calling oidmap_clear(&missing_objects, true).\n\nIntroduce a dedicated free callback and pass it to\noidmap_clear_with_free(), consolidating entry teardown into a\nsingle place and making cleanup semantics explicit.\n\nSigned-off-by: Seyi Kuforiji \n---\n builtin/rev-list.c | 15 ++++++++++++---\n 1 file changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex ddea8aa251..ab5f69826c 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -88,9 +88,19 @@ static int arg_print_omitted; /* print objects omitted by filter */\n \n struct missing_objects_map_entry {\n \tstruct oidmap_entry entry;\n-\tconst char *path;\n+\tchar *path;\n \tunsigned type;\n };\n+\n+static void free_missing_objects_entry(void *e)\n+{\n+\tstruct missing_objects_map_entry *entry =\n+\t\tcontainer_of(e, struct missing_objects_map_entry, entry);\n+\n+\tfree(entry->path);\n+\tfree(entry);\n+}\n+\n static struct oidmap missing_objects;\n enum missing_action {\n \tMA_ERROR = 0, /* fail if any missing objects are encountered */\n@@ -935,10 +945,9 @@ int cmd_rev_list(int argc,\n \t\twhile ((entry = oidmap_iter_next(&iter))) {\n \t\t\tprint_missing_object(entry, arg_missing_action ==\n \t\t\t\t\t\t\t MA_PRINT_INFO);\n-\t\t\tfree((void *)entry->path);\n \t\t}\n \n-\t\toidmap_clear(&missing_objects, true);\n+\t\toidmap_clear_with_free(&missing_objects, free_missing_objects_entry);\n \t}\n \n \tstop_progress(&progress);\n-- \n2.43.0\n\n"},{"id":"537599","messageId":"20260302200018.75731-4-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260302200018.75731-1-kuforiji98@gmail.com","subject":"[PATCH v2 3/5] list-objects-filter: use oidmap_clear_with_free() for cleanup","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-03-02T20:00:15Z","receivedAt":"2026-03-02T20:01:18Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nReplace the use of oidmap_clear(&seen_at_depth, 1) in\nfilter_trees_free() with oidmap_clear_with_free().\n\nThe seen_at_depth map stores heap-allocated struct\nseen_map_entry objects. Previously, passing 1 relied on\noidmap_clear() internally calling free() on each entry.\n\nConvert this to the explicit oidmap_clear_with_free() API\nand provide a typed free_seen_map_entry() helper to free\neach container entry.\n\nThis makes the ownership and cleanup policy explicit and\nremoves reliance on the legacy boolean free_entries\nparameter.\n\nSigned-off-by: Seyi Kuforiji \n---\n list-objects-filter.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/list-objects-filter.c b/list-objects-filter.c\nindex 78316e7f90..0038bfaac5 100644\n--- a/list-objects-filter.c\n+++ b/list-objects-filter.c\n@@ -143,6 +143,13 @@ struct seen_map_entry {\n \tsize_t depth;\n };\n \n+static void free_seen_map_entry(void *e)\n+{\n+\tstruct seen_map_entry *entry =\n+\t\tcontainer_of(e, struct seen_map_entry, base);\n+\tfree(entry);\n+}\n+\n /* Returns 1 if the oid was in the omits set before it was invoked. */\n static int filter_trees_update_omits(\n \tstruct object *obj,\n@@ -244,7 +251,7 @@ static void filter_trees_free(void *filter_data) {\n \tstruct filter_trees_depth_data *d = filter_data;\n \tif (!d)\n \t\treturn;\n-\toidmap_clear(&d->seen_at_depth, 1);\n+\toidmap_clear_with_free(&d->seen_at_depth, free_seen_map_entry);\n \tfree(d);\n }\n \n-- \n2.43.0\n\n"},{"id":"537600","messageId":"20260302200018.75731-5-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260302200018.75731-1-kuforiji98@gmail.com","subject":"[PATCH v2 4/5] odb: use oidmap_clear_with_free() to release replace_map entries","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-03-02T20:00:16Z","receivedAt":"2026-03-02T20:01:23Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nReplace the direct oidmap_clear() call in odb_free() with\noidmap_clear_with_free(), and introduce a free_replace_map_entry()\nhelper to properly free each struct replace_object stored in the map.\n\nThis centralizes cleanup logic and ensures entries are released\ncorrectly via a dedicated callback.\n\nSigned-off-by: Seyi Kuforiji \n---\n odb.c | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/odb.c b/odb.c\nindex 1679cc0465..8ca497203f 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -14,6 +14,7 @@\n #include \"object-file-convert.h\"\n #include \"object-file.h\"\n #include \"odb.h\"\n+#include \"oidmap.h\"\n #include \"packfile.h\"\n #include \"path.h\"\n #include \"promisor-remote.h\"\n@@ -1089,6 +1090,13 @@ void odb_close(struct object_database *o)\n \tclose_commit_graph(o);\n }\n \n+static void free_replace_map_entry(void *e)\n+{\n+\tstruct replace_object *entry =\n+\t\tcontainer_of(e, struct replace_object, original);\n+\tfree(entry);\n+}\n+\n static void odb_free_sources(struct object_database *o)\n {\n \twhile (o->sources) {\n@@ -1109,7 +1117,8 @@ void odb_free(struct object_database *o)\n \n \tfree(o->alternate_db);\n \n-\toidmap_clear(&o->replace_map, 1);\n+\tif (o->replace_map_initialized)\n+\t\toidmap_clear_with_free(&o->replace_map, free_replace_map_entry);\n \tpthread_mutex_destroy(&o->replace_mutex);\n \n \todb_close(o);\n-- \n2.43.0\n\n"},{"id":"537601","messageId":"20260302200018.75731-6-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260302200018.75731-1-kuforiji98@gmail.com","subject":"[PATCH v2 5/5] sequencer: use oidmap_clear_with_free() for string_entry cleanup","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-03-02T20:00:17Z","receivedAt":"2026-03-02T20:01:31Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nSwitch cleanup of the string_entry oidmap to\noidmap_clear_with_free() and introduce a free_string_entry()\nhelper to properly free each allocated struct string_entry.\n\nThis aligns with the ongoing migration to use the callback-based\noidmap cleanup API.\n\nSigned-off-by: Seyi Kuforiji \n---\n sequencer.c | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex a3eb39bb25..75ef2ace4f 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -5654,6 +5654,12 @@ struct string_entry {\n \tchar string[FLEX_ARRAY];\n };\n \n+static void free_string_entry(void *e)\n+{\n+\tstruct string_entry *entry = container_of(e, struct string_entry, entry);\n+\tfree(entry);\n+}\n+\n struct label_state {\n \tstruct oidmap commit2label;\n \tstruct hashmap labels;\n@@ -6044,8 +6050,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \toidset_clear(&interesting);\n \toidset_clear(&child_seen);\n \toidset_clear(&shown);\n-\toidmap_clear(&commit2todo, 1);\n-\toidmap_clear(&state.commit2label, 1);\n+\toidmap_clear_with_free(&commit2todo, free_string_entry);\n+\toidmap_clear_with_free(&state.commit2label, free_string_entry);\n \thashmap_clear_and_free(&state.labels, struct labels_entry, entry);\n \tstrbuf_release(&state.buf);\n \n-- \n2.43.0\n\n"},{"id":"537621","messageId":"xmqqfr6hyior.fsf@gitster.g","threadId":"65093","inReplyTo":"20260302200018.75731-2-kuforiji98@gmail.com","subject":"Re: [PATCH v2 1/5] oidmap: make entry cleanup explicit in oidmap_clear","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T22:23:32Z","receivedAt":"2026-03-02T22:23:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seyi Kuforiji writes:\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\nMade me briefly wonder if passing \"void free(void *)\" or NULL as\noidmap_free_fn would be flagged by the compilers as suspicious, but \n\"typedef void (*oidmap_free_fn)(void *);\" is obviously compatible\nwith both, so it is good.\n\n> +{\n> +\tstruct hashmap_iter iter;\n> +\tstruct hashmap_entry *e;\n> +\n> +\tif (!map || !map->map.cmpfn)\n> \t\treturn;\n\nThe first half prepares our oidmap_clear() to be fed a NULL map, but\nwhat about the new condition? Where did it come from? What makes\nit suddenly necessary that in order to \"clear\" an oidmap you already\nhave to have defined cmpfn?\n\n> -\t/* TODO: make oidmap itself not depend on struct layouts */\n> -\thashmap_clear_(&map->map, free_entries ? 0 : -1);\n\nThis is now achieved by the use of container_of(), right? Nice.\n\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"},{"id":"537622","messageId":"xmqqbjh5yik7.fsf@gitster.g","threadId":"65093","inReplyTo":"20260302200018.75731-3-kuforiji98@gmail.com","subject":"Re: [PATCH v2 2/5] builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T22:26:16Z","receivedAt":"2026-03-02T22:26:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seyi Kuforiji writes:\n\n> From: Seyi Kufoiji \n>\n> As part of the conversion away from oidmap_clear(), switch the\n> missing_objects map to use oidmap_clear_with_free().\n>\n> missing_objects stores struct missing_objects_map_entry instances,\n> which own an xstrdup()'d path string in addition to the container\n> struct itself. Previously, rev-list manually freed entry->path\n> before calling oidmap_clear(&missing_objects, true).\n>\n> Introduce a dedicated free callback and pass it to\n> oidmap_clear_with_free(), consolidating entry teardown into a\n> single place and making cleanup semantics explicit.\n>\n> Signed-off-by: Seyi Kuforiji \n> ---\n> builtin/rev-list.c | 15 ++++++++++++---\n> 1 file changed, 12 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> index ddea8aa251..ab5f69826c 100644\n> --- a/builtin/rev-list.c\n> +++ b/builtin/rev-list.c\n> @@ -88,9 +88,19 @@ static int arg_print_omitted; /* print objects omitted by filter */\n> \n> struct missing_objects_map_entry {\n> \tstruct oidmap_entry entry;\n> -\tconst char *path;\n> +\tchar *path;\n> \tunsigned type;\n> };\n> +\n> +static void free_missing_objects_entry(void *e)\n> +{\n> +\tstruct missing_objects_map_entry *entry =\n> +\t\tcontainer_of(e, struct missing_objects_map_entry, entry);\n> +\n> +\tfree(entry->path);\n> +\tfree(entry);\n> +}\n\nUnlike the previous iteration, this makes the claim very believable\nthat it makes the code easier to follow to have the shell and its\ncontents released together in a single function.\n\nLooking good.\n\n> static struct oidmap missing_objects;\n> enum missing_action {\n> \tMA_ERROR = 0, /* fail if any missing objects are encountered */\n> @@ -935,10 +945,9 @@ int cmd_rev_list(int argc,\n> \t\twhile ((entry = oidmap_iter_next(&iter))) {\n> \t\t\tprint_missing_object(entry, arg_missing_action ==\n> \t\t\t\t\t\t\t MA_PRINT_INFO);\n> -\t\t\tfree((void *)entry->path);\n> \t\t}\n> \n> -\t\toidmap_clear(&missing_objects, true);\n> +\t\toidmap_clear_with_free(&missing_objects, free_missing_objects_entry);\n> \t}\n> \n> \tstop_progress(&progress);\n"},{"id":"537623","messageId":"xmqq7brtyids.fsf@gitster.g","threadId":"65093","inReplyTo":"20260302200018.75731-4-kuforiji98@gmail.com","subject":"Re: [PATCH v2 3/5] list-objects-filter: use oidmap_clear_with_free() for cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T22:30:07Z","receivedAt":"2026-03-02T22:30:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seyi Kuforiji writes:\n\n> diff --git a/list-objects-filter.c b/list-objects-filter.c\n> index 78316e7f90..0038bfaac5 100644\n> --- a/list-objects-filter.c\n> +++ b/list-objects-filter.c\n> @@ -143,6 +143,13 @@ struct seen_map_entry {\n> \tsize_t depth;\n> };\n> \n> +static void free_seen_map_entry(void *e)\n> +{\n> +\tstruct seen_map_entry *entry =\n> +\t\tcontainer_of(e, struct seen_map_entry, base);\n> +\tfree(entry);\n> +}\n\nAs there is *no* extra resources held in seen_map_entry other than\nthe shell itself, this step alone does not make the code any clearer\nto follow. But if we are going to add new members to the structure\nin the future, the story will change and we'll leap the same benefit\nas we saw in [PATCH v2 2/5].\n\n> @@ -244,7 +251,7 @@ static void filter_trees_free(void *filter_data) {\n> \tstruct filter_trees_depth_data *d = filter_data;\n> \tif (!d)\n> \t\treturn;\n> -\toidmap_clear(&d->seen_at_depth, 1);\n> +\toidmap_clear_with_free(&d->seen_at_depth, free_seen_map_entry);\n> \tfree(d);\n> }\n"},{"id":"537624","messageId":"xmqq1pi1yi4s.fsf@gitster.g","threadId":"65093","inReplyTo":"20260302200018.75731-5-kuforiji98@gmail.com","subject":"Re: [PATCH v2 4/5] odb: use oidmap_clear_with_free() to release replace_map entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T22:35:31Z","receivedAt":"2026-03-02T22:35:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seyi Kuforiji writes:\n\n> +static void free_replace_map_entry(void *e)\n> +{\n> +\tstruct replace_object *entry =\n> +\t\tcontainer_of(e, struct replace_object, original);\n> +\tfree(entry);\n> +}\n> +\n\nThe same comment as [PATCH v2 3/5].\n\n\n> @@ -1109,7 +1117,8 @@ void odb_free(struct object_database *o)\n> \n> \tfree(o->alternate_db);\n> \n> -\toidmap_clear(&o->replace_map, 1);\n> +\tif (o->replace_map_initialized)\n> +\t\toidmap_clear_with_free(&o->replace_map, free_replace_map_entry);\n\nIt is a bit unfortunate that we need to know how o->replace_map is\ninitialized and maintained. I wondered if we can do this without\npeeking into o->replace_map_initialized, but o->replace_map is\nalready an instance of the map, not a pointer that points at a\nlazily initialized instance of a map, so that cannot be done (and we\nwould not have replace_map_initialized member in the object_database\nstruct in the first place, if we can tell if o.replace_map needs\nclearing by simply looking at it).\n\nSo, all OK, I guess.\n\n> \tpthread_mutex_destroy(&o->replace_mutex);\n> \n> \todb_close(o);\n"},{"id":"537625","messageId":"xmqqwlztx3f7.fsf@gitster.g","threadId":"65093","inReplyTo":"20260302200018.75731-6-kuforiji98@gmail.com","subject":"Re: [PATCH v2 5/5] sequencer: use oidmap_clear_with_free() for string_entry cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T22:38:36Z","receivedAt":"2026-03-02T22:38:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seyi Kuforiji writes:\n\n> From: Seyi Kufoiji \n>\n> Switch cleanup of the string_entry oidmap to\n> oidmap_clear_with_free() and introduce a free_string_entry()\n> helper to properly free each allocated struct string_entry.\n>\n> This aligns with the ongoing migration to use the callback-based\n> oidmap cleanup API.\n>\n> Signed-off-by: Seyi Kuforiji \n> ---\n> sequencer.c | 10 ++++++++--\n> 1 file changed, 8 insertions(+), 2 deletions(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index a3eb39bb25..75ef2ace4f 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -5654,6 +5654,12 @@ struct string_entry {\n> \tchar string[FLEX_ARRAY];\n> };\n> \n> +static void free_string_entry(void *e)\n> +{\n> +\tstruct string_entry *entry = container_of(e, struct string_entry, entry);\n> +\tfree(entry);\n> +}\n\nExactly the same comment applies to this step as [PATCH v2 3/5].\n\nIn other words, with the current codebase, these three steps in the\ncontext of the current code are uninteresting with little value, but\nif we ever add a member to these entries that hold their own\nresources, it would become easier to manage the lifetime rules of\nthem.\n\n> @@ -6044,8 +6050,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n> \toidset_clear(&interesting);\n> \toidset_clear(&child_seen);\n> \toidset_clear(&shown);\n> -\toidmap_clear(&commit2todo, 1);\n> -\toidmap_clear(&state.commit2label, 1);\n> +\toidmap_clear_with_free(&commit2todo, free_string_entry);\n> +\toidmap_clear_with_free(&state.commit2label, free_string_entry);\n> \thashmap_clear_and_free(&state.labels, struct labels_entry, entry);\n> \tstrbuf_release(&state.buf);\n"},{"id":"537750","messageId":"aafX5CmP82WYFyIb@pks.im","threadId":"65093","inReplyTo":"20260302200018.75731-3-kuforiji98@gmail.com","subject":"Re: [PATCH v2 2/5] builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-04T06:57:40Z","receivedAt":"2026-03-04T06:57:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Mar 02, 2026 at 09:00:14PM +0100, Seyi Kuforiji wrote:\n> diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> index ddea8aa251..ab5f69826c 100644\n> --- a/builtin/rev-list.c\n> +++ b/builtin/rev-list.c\n> @@ -88,9 +88,19 @@ static int arg_print_omitted; /* print objects omitted by filter */\n> \n> struct missing_objects_map_entry {\n> \tstruct oidmap_entry entry;\n> -\tconst char *path;\n> +\tchar *path;\n> \tunsigned type;\n> };\n> +\n> +static void free_missing_objects_entry(void *e)\n\nNit: this should be called `missing_objects_map_entry_free()` according\nto our coding guidelines.\n\nPatrick\n"},{"id":"537751","messageId":"aafX6qva_badx_RM@pks.im","threadId":"65093","inReplyTo":"xmqq7brtyids.fsf@gitster.g","subject":"Re: [PATCH v2 3/5] list-objects-filter: use oidmap_clear_with_free() for cleanup","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-04T06:57:46Z","receivedAt":"2026-03-04T06:57:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Mar 02, 2026 at 02:30:07PM -0800, Junio C Hamano wrote:\n> Seyi Kuforiji writes:\n> \n> > diff --git a/list-objects-filter.c b/list-objects-filter.c\n> > index 78316e7f90..0038bfaac5 100644\n> > --- a/list-objects-filter.c\n> > +++ b/list-objects-filter.c\n> > @@ -143,6 +143,13 @@ struct seen_map_entry {\n> > \tsize_t depth;\n> > };\n> > \n> > +static void free_seen_map_entry(void *e)\n> > +{\n> > +\tstruct seen_map_entry *entry =\n> > +\t\tcontainer_of(e, struct seen_map_entry, base);\n> > +\tfree(entry);\n> > +}\n> \n> As there is *no* extra resources held in seen_map_entry other than\n> the shell itself, this step alone does not make the code any clearer\n> to follow. But if we are going to add new members to the structure\n> in the future, the story will change and we'll leap the same benefit\n> as we saw in [PATCH v2 2/5].\n\nAgreed. But I think with the current status quo I'd rather drop this\npatch though as it may otherwise make the reader scratch their head why\nwe do the exercise in the first place.\n\nPatrick\n"},{"id":"537752","messageId":"aafX7_BqIYDfXQtN@pks.im","threadId":"65093","inReplyTo":"xmqqwlztx3f7.fsf@gitster.g","subject":"Re: [PATCH v2 5/5] sequencer: use oidmap_clear_with_free() for string_entry cleanup","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-04T06:57:51Z","receivedAt":"2026-03-04T06:57:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Mar 02, 2026 at 02:38:36PM -0800, Junio C Hamano wrote:\n> Seyi Kuforiji writes:\n> \n> > From: Seyi Kufoiji \n> >\n> > Switch cleanup of the string_entry oidmap to\n> > oidmap_clear_with_free() and introduce a free_string_entry()\n> > helper to properly free each allocated struct string_entry.\n> >\n> > This aligns with the ongoing migration to use the callback-based\n> > oidmap cleanup API.\n> >\n> > Signed-off-by: Seyi Kuforiji \n> > ---\n> > sequencer.c | 10 ++++++++--\n> > 1 file changed, 8 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/sequencer.c b/sequencer.c\n> > index a3eb39bb25..75ef2ace4f 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -5654,6 +5654,12 @@ struct string_entry {\n> > \tchar string[FLEX_ARRAY];\n> > };\n> > \n> > +static void free_string_entry(void *e)\n> > +{\n> > +\tstruct string_entry *entry = container_of(e, struct string_entry, entry);\n> > +\tfree(entry);\n> > +}\n> \n> Exactly the same comment applies to this step as [PATCH v2 3/5].\n> \n> In other words, with the current codebase, these three steps in the\n> context of the current code are uninteresting with little value, but\n> if we ever add a member to these entries that hold their own\n> resources, it would become easier to manage the lifetime rules of\n> them.\n\nPersonally I'd lean towards keeping the first two patches and drop the\nremaining ones though. I think it makes the code harder to understand to\nconvert all callsites of `oidmap_clear()`, even if it doesn't actually\nprovide a benefit.\n\nWe can still convert callsites to use `oidmap_clear_with_free()` in case\nthey grow additional allocations per entry that we'll have to care\nabout.\n\nThanks!\n\nPatrick\n"},{"id":"537785","messageId":"xmqqjyvra9xg.fsf@gitster.g","threadId":"65093","inReplyTo":"aafX6qva_badx_RM@pks.im","subject":"Re: [PATCH v2 3/5] list-objects-filter: use oidmap_clear_with_free() for cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-04T15:31:07Z","receivedAt":"2026-03-04T15:31:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Agreed. But I think with the current status quo I'd rather drop this\n> patch though as it may otherwise make the reader scratch their head why\n> we do the exercise in the first place.\n\nI do not think too strongly either way myself, but you may be right.\n\nUnless we are dropping the \"we optionally let you free the shell\"\ntraditional interface, it is of questionable value to use the new\ninterface.\n\nThanks.\n\n"},{"id":"537832","messageId":"CAGedMte09S1FE2nX5SnamzqZyMGfme-kL0skZ+e+st-b2HbQMA@mail.gmail.com","threadId":"65093","inReplyTo":"xmqqjyvra9xg.fsf@gitster.g","subject":"Re: [PATCH v2 3/5] list-objects-filter: use oidmap_clear_with_free() for cleanup","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-03-04T19:29:01Z","receivedAt":"2026-03-04T19:29:15Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"On Wed, 4 Mar 2026 at 16:31, Junio C Hamano wrote:\n>\n> Patrick Steinhardt writes:\n>\n> > Agreed. But I think with the current status quo I'd rather drop this\n> > patch though as it may otherwise make the reader scratch their head why\n> > we do the exercise in the first place.\n>\n> I do not think too strongly either way myself, but you may be right.\n>\n> Unless we are dropping the \"we optionally let you free the shell\"\n> traditional interface, it is of questionable value to use the new\n> interface.\n>\n> Thanks.\n>\n\nHello\n\nThank you so much for the reviews.\n\nI'll send a new version dropping the [PATCH 3/5].\n\nThanks,\nSeyi\n"},{"id":"537834","messageId":"xmqqcy1j72y7.fsf@gitster.g","threadId":"65093","inReplyTo":"CAGedMte09S1FE2nX5SnamzqZyMGfme-kL0skZ+e+st-b2HbQMA@mail.gmail.com","subject":"Re: [PATCH v2 3/5] list-objects-filter: use oidmap_clear_with_free() for cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-04T20:30:08Z","receivedAt":"2026-03-04T20:30:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seyi Kuforiji writes:\n\n> On Wed, 4 Mar 2026 at 16:31, Junio C Hamano wrote:\n>>\n>> Patrick Steinhardt writes:\n>>\n>> > Agreed. But I think with the current status quo I'd rather drop this\n>> > patch though as it may otherwise make the reader scratch their head why\n>> > we do the exercise in the first place.\n>>\n>> I do not think too strongly either way myself, but you may be right.\n>>\n>> Unless we are dropping the \"we optionally let you free the shell\"\n>> traditional interface, it is of questionable value to use the new\n>> interface.\n>>\n>> Thanks.\n>>\n>\n> Hello\n>\n> Thank you so much for the reviews.\n>\n> I'll send a new version dropping the [PATCH 3/5].\n\nI thought that Patrick wants to see only [1/5] and [2/5], discarding\nthe rest (i.e. 3/5, 4/5, and 5/5). If that is the plan, I do not\nthink we need any resend.\n\nThanks.\n"},{"id":"537843","messageId":"xmqqzf4n5kzu.fsf@gitster.g","threadId":"65093","inReplyTo":"xmqqcy1j72y7.fsf@gitster.g","subject":"Re: [PATCH v2 3/5] list-objects-filter: use oidmap_clear_with_free() for cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-04T21:43:17Z","receivedAt":"2026-03-04T21:43:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Seyi Kuforiji writes:\n>\n>> On Wed, 4 Mar 2026 at 16:31, Junio C Hamano wrote:\n>>>\n>>> Patrick Steinhardt writes:\n>>>\n>>> > Agreed. But I think with the current status quo I'd rather drop this\n>>> > patch though as it may otherwise make the reader scratch their head why\n>>> > we do the exercise in the first place.\n>>>\n>>> I do not think too strongly either way myself, but you may be right.\n>>>\n>>> Unless we are dropping the \"we optionally let you free the shell\"\n>>> traditional interface, it is of questionable value to use the new\n>>> interface.\n>>>\n>>> Thanks.\n>>>\n>>\n>> Hello\n>>\n>> Thank you so much for the reviews.\n>>\n>> I'll send a new version dropping the [PATCH 3/5].\n>\n> I thought that Patrick wants to see only [1/5] and [2/5], discarding\n> the rest (i.e. 3/5, 4/5, and 5/5). If that is the plan, I do not\n> think we need any resend.\n\nAh, in https://lore.kernel.org/git/aafX5CmP82WYFyIb@pks.im/ he wants\nthe callback to be renamed, so we do need a new iteration (v3). I\nstill think that if you are to drop [3/5], then [4/5] and [5/5]\nshould also be dropped, leaving only the first two patches.\n\nThanks.\n\n"},{"id":"537910","messageId":"20260305100526.102130-1-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260302200018.75731-1-kuforiji98@gmail.com","subject":"[PATCH v3 0/2] oidmap: migrate cleanup to oidmap_clear_with_free()","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-03-05T10:05:24Z","receivedAt":"2026-03-05T10:05:58Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Hi,\n\nThis series replaces oidmap_clear(map, 1) with\noidmap_clear_with_free() and introduces explicit free callbacks\nat the remaining call sites.\n\nThe old boolean-based API implicitly assumed plain free(),\nwhich obscures ownership semantics and does not work well\nwhen oidmap_entry is embedded inside larger structures.\nThe callback-based API makes cleanup explicit and type-safe,\nand avoids relying on hidden assumptions about allocation.\n\nThis is used in subsequent commits to adequately cleanup all\nusage site.\n\nChanges in v3:\n - rename funtion to meet guidlines\n - drop [PATCH 3/5]\n\nThanks\nSeyi\n\nSeyi Kufoiji (2):\n oidmap: make entry cleanup explicit in oidmap_clear\n builtin/rev-list: migrate missing_objects cleanup to\n oidmap_clear_with_free()\n\n builtin/rev-list.c | 15 ++++++++++++---\n oidmap.c | 23 ++++++++++++++++++++---\n oidmap.h | 15 +++++++++++++++\n t/unit-tests/u-oidmap.c | 41 +++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 88 insertions(+), 6 deletions(-)\n\nRange-diff against v2:\n1: 1d544ef7d2 = 1: a050491441 oidmap: make entry cleanup explicit in oidmap_clear\n2: f2c3a699bd ! 2: b592d765e3 builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free()\n @@ builtin/rev-list.c: static int arg_print_omitted; /* print objects omitted by fi\n \tunsigned type;\n };\n +\n -+static void free_missing_objects_entry(void *e)\n ++static void missing_objects_map_entry_free(void *e)\n +{\n +\tstruct missing_objects_map_entry *entry =\n +\t\tcontainer_of(e, struct missing_objects_map_entry, entry);\n @@ builtin/rev-list.c: int cmd_rev_list(int argc,\n \t\t}\n \n -\t\toidmap_clear(&missing_objects, true);\n -+\t\toidmap_clear_with_free(&missing_objects, free_missing_objects_entry);\n ++\t\toidmap_clear_with_free(&missing_objects, missing_objects_map_entry_free);\n \t}\n \n \tstop_progress(&progress);\n3: a4e426bcca < -: ---------- list-objects-filter: use oidmap_clear_with_free() for cleanup\n4: 4116e5491d < -: ---------- odb: use oidmap_clear_with_free() to release replace_map entries\n5: ad1f776a19 < -: ---------- sequencer: use oidmap_clear_with_free() for string_entry cleanup\n-- \n2.43.0\n\n"},{"id":"537911","messageId":"20260305100526.102130-2-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260305100526.102130-1-kuforiji98@gmail.com","subject":"[PATCH v3 1/2] oidmap: make entry cleanup explicit in oidmap_clear","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-03-05T10:05:25Z","receivedAt":"2026-03-05T10:06:03Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nReplace 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"},{"id":"537912","messageId":"20260305100526.102130-3-kuforiji98@gmail.com","threadId":"65093","inReplyTo":"20260305100526.102130-1-kuforiji98@gmail.com","subject":"[PATCH v3 2/2] builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free()","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2026-03-05T10:05:26Z","receivedAt":"2026-03-05T10:06:10Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"From: Seyi Kufoiji \n\nAs part of the conversion away from oidmap_clear(), switch the\nmissing_objects map to use oidmap_clear_with_free().\n\nmissing_objects stores struct missing_objects_map_entry instances,\nwhich own an xstrdup()'d path string in addition to the container\nstruct itself. Previously, rev-list manually freed entry->path\nbefore calling oidmap_clear(&missing_objects, true).\n\nIntroduce a dedicated free callback and pass it to\noidmap_clear_with_free(), consolidating entry teardown into a\nsingle place and making cleanup semantics explicit.\n\nSigned-off-by: Seyi Kuforiji \n---\n builtin/rev-list.c | 15 ++++++++++++---\n 1 file changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex ddea8aa251..854d82ece3 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -88,9 +88,19 @@ static int arg_print_omitted; /* print objects omitted by filter */\n \n struct missing_objects_map_entry {\n \tstruct oidmap_entry entry;\n-\tconst char *path;\n+\tchar *path;\n \tunsigned type;\n };\n+\n+static void missing_objects_map_entry_free(void *e)\n+{\n+\tstruct missing_objects_map_entry *entry =\n+\t\tcontainer_of(e, struct missing_objects_map_entry, entry);\n+\n+\tfree(entry->path);\n+\tfree(entry);\n+}\n+\n static struct oidmap missing_objects;\n enum missing_action {\n \tMA_ERROR = 0, /* fail if any missing objects are encountered */\n@@ -935,10 +945,9 @@ int cmd_rev_list(int argc,\n \t\twhile ((entry = oidmap_iter_next(&iter))) {\n \t\t\tprint_missing_object(entry, arg_missing_action ==\n \t\t\t\t\t\t\t MA_PRINT_INFO);\n-\t\t\tfree((void *)entry->path);\n \t\t}\n \n-\t\toidmap_clear(&missing_objects, true);\n+\t\toidmap_clear_with_free(&missing_objects, missing_objects_map_entry_free);\n \t}\n \n \tstop_progress(&progress);\n-- \n2.43.0\n\n"},{"id":"537974","messageId":"aamSznHdv4g97R6C@pks.im","threadId":"65093","inReplyTo":"20260305100526.102130-1-kuforiji98@gmail.com","subject":"Re: [PATCH v3 0/2] oidmap: migrate cleanup to oidmap_clear_with_free()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-05T14:27:26Z","receivedAt":"2026-03-05T14:27:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Mar 05, 2026 at 11:05:24AM +0100, Seyi Kuforiji wrote:\n> Hi,\n> \n> This series replaces oidmap_clear(map, 1) with\n> oidmap_clear_with_free() and introduces explicit free callbacks\n> at the remaining call sites.\n> \n> The old boolean-based API implicitly assumed plain free(),\n> which obscures ownership semantics and does not work well\n> when oidmap_entry is embedded inside larger structures.\n> The callback-based API makes cleanup explicit and type-safe,\n> and avoids relying on hidden assumptions about allocation.\n> \n> This is used in subsequent commits to adequately cleanup all\n> usage site.\n> \n> Changes in v3:\n> - rename funtion to meet guidlines\n> - drop [PATCH 3/5]\n\nThanks, this version looks good to me!\n\nPatrick\n"},{"id":"537990","messageId":"xmqq1phy3x2h.fsf@gitster.g","threadId":"65093","inReplyTo":"20260305100526.102130-1-kuforiji98@gmail.com","subject":"Re: [PATCH v3 0/2] oidmap: migrate cleanup to oidmap_clear_with_free()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-05T19:17:42Z","receivedAt":"2026-03-05T19:17:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seyi Kuforiji writes:\n\n> Range-diff against v2:\n> 1: 1d544ef7d2 = 1: a050491441 oidmap: make entry cleanup explicit in oidmap_clear\n> 2: f2c3a699bd ! 2: b592d765e3 builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free()\n> @@ builtin/rev-list.c: static int arg_print_omitted; /* print objects omitted by fi\n> \tunsigned type;\n> };\n> +\n> -+static void free_missing_objects_entry(void *e)\n> ++static void missing_objects_map_entry_free(void *e)\n> +{\n> +\tstruct missing_objects_map_entry *entry =\n> +\t\tcontainer_of(e, struct missing_objects_map_entry, entry);\n> @@ builtin/rev-list.c: int cmd_rev_list(int argc,\n> \t\t}\n> \n> -\t\toidmap_clear(&missing_objects, true);\n> -+\t\toidmap_clear_with_free(&missing_objects, free_missing_objects_entry);\n> ++\t\toidmap_clear_with_free(&missing_objects, missing_objects_map_entry_free);\n> \t}\n> \n> \tstop_progress(&progress);\n> 3: a4e426bcca < -: ---------- list-objects-filter: use oidmap_clear_with_free() for cleanup\n> 4: 4116e5491d < -: ---------- odb: use oidmap_clear_with_free() to release replace_map entries\n> 5: ad1f776a19 < -: ---------- sequencer: use oidmap_clear_with_free() for string_entry cleanup\n\nI think these cover everything Patrick pointed out, and I agree with\nwhat these remaining two patches do. Will queue.\n\nLet me mark the topic for 'next'.\n\nThanks.\n"}]}