Re: [PATCH v2 5/5] sequencer: use oidmap_clear_with_free() for string_entry cleanup

[Junio C Hamano <gitster@pobox.com>]

Seyi Kuforiji <kuforiji98@gmail.com> writes:

> From: Seyi Kufoiji <kuforiji98@gmail.com>
>
> Switch cleanup of the string_entry oidmap to
> oidmap_clear_with_free() and introduce a free_string_entry()
> helper to properly free each allocated struct string_entry.
>
> This aligns with the ongoing migration to use the callback-based
> oidmap cleanup API.
>
> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
> ---
>  sequencer.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index a3eb39bb25..75ef2ace4f 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -5654,6 +5654,12 @@ struct string_entry {
>  	char string[FLEX_ARRAY];
>  };
>  
> +static void free_string_entry(void *e)
> +{
> +	struct string_entry *entry = container_of(e, struct string_entry, entry);
> +	free(entry);
> +}

Exactly the same comment applies to this step as [PATCH v2 3/5].

In other words, with the current codebase, these three steps in the
context of the current code are uninteresting with little value, but
if we ever add a member to these entries that hold their own
resources, it would become easier to manage the lifetime rules of
them.

> @@ -6044,8 +6050,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,
>  	oidset_clear(&interesting);
>  	oidset_clear(&child_seen);
>  	oidset_clear(&shown);
> -	oidmap_clear(&commit2todo, 1);
> -	oidmap_clear(&state.commit2label, 1);
> +	oidmap_clear_with_free(&commit2todo, free_string_entry);
> +	oidmap_clear_with_free(&state.commit2label, free_string_entry);
>  	hashmap_clear_and_free(&state.labels, struct labels_entry, entry);
>  	strbuf_release(&state.buf);