Re: [PATCH v6 05/17] hook API: add a run_hooks_l() wrapper

[Emily Shaffer <emilyshaffer@google.com>]

On Wed, Dec 22, 2021 at 04:59:31AM +0100, Ævar Arnfjörð Bjarmason wrote:
> 
> Add a run_hooks_l() wrapper, we'll use it in subsequent commits for
> the simple cases of wanting to run a single hook under a given name
> along with a list of arguments.

I've got a slight allergy to varargs, just as personal preference, but
there's no denying that this makes the following callsite conversions
extremely mechanical and easy to review.

Does this calling convention make it easier for us to try and inline
results from something like oid_to_hex(), which may become stale,
compared to needing to put together a struct with args? I think probably
it's fine - because strvec_push() creates a copy, and run_hooks_l() is
not something we'd want to call multiple times in parallel (I hope).

Anyway,
Reviewed-by: Emily Shaffer <emilyshaffer@google.com>

> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  hook.c | 14 ++++++++++++++
>  hook.h | 10 ++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/hook.c b/hook.c
> index d67a114e62d..1ad123422b2 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -149,3 +149,17 @@ int run_hooks(const char *hook_name)
>  
>  	return run_hooks_opt(hook_name, &opt);
>  }
> +
> +int run_hooks_l(const char *hook_name, ...)
> +{
> +	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
> +	va_list ap;
> +	const char *arg;
> +
> +	va_start(ap, hook_name);
> +	while ((arg = va_arg(ap, const char *)))
> +		strvec_push(&opt.args, arg);
> +	va_end(ap);
> +
> +	return run_hooks_opt(hook_name, &opt);
> +}
> diff --git a/hook.h b/hook.h
> index 9c358789958..54528395953 100644
> --- a/hook.h
> +++ b/hook.h
> @@ -54,4 +54,14 @@ int run_hooks_opt(const char *hook_name, struct run_hooks_opt *options);
>   * run_hooks_opt" initialized with "RUN_HOOKS_OPT_INIT".
>   */
>  int run_hooks(const char *hook_name);
> +
> +/**
> + * Like run_hooks(), a wrapper for run_hooks_opt().
> + *
> + * In addition to the wrapping behavior provided by run_hooks(), this
> + * wrapper takes a list of strings terminated by a NULL
> + * argument. These things will be used as positional arguments to the
> + * hook. This function behaves like the old run_hook_le() API.
> + */
> +int run_hooks_l(const char *hook_name, ...);
>  #endif
> -- 
> 2.34.1.1146.gb52885e7c44
>