Subject: Re: [PATCH v2 05/10] hook: replace hook_list_clear() -> string_list_clear_func()

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 24 Mar 2026 09:37:52 +0100

On Fri, Mar 20, 2026 at 01:52:06PM +0200, Adrian Ratiu wrote:
> diff --git a/hook.c b/hook.c
> index 6dfaa7e9b1..f6bb1999ae 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -52,8 +52,14 @@ const char *find_hook(struct repository *r, const char *name)
>  	return path.buf;
>  }
>  
> -static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
> +/*
> + * Frees a struct hook stored as the util pointer of a string_list_item.
> + * Suitable for use as a string_list_clear_func_t callback.
> + */

This comment should probably live in the header. I also wonder whether
this wrapper isn't a bit too specific to freeing hooks with a string
list. Maybe it would be preferable to expose a "proper" `hook_free()`
function that only takes a hook, and then provide a small wrapper
function for freeing in the string list?

If so it feels like we're going a bit full circle though. Maybe the
original code wasn't all that bad in the first place?

Patrick