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

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 24, 2026, 08:37 UTC
Message-ID: <acJNYPgOSO86hZYq@pks.im>
In-Reply-To: <20260320115211.177351-6-adrian.ratiu@collabora.com>

On Fri, Mar 20, 2026 at 01:52:06PM +0200, Adrian Ratiu wrote:

Show 13 quoted lines

> 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