From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 24 Mar 2026 08:37:47 GMT
Subject: Re: [PATCH v2 02/10] hook: fix minor style issues
Message-ID: <acJNW0m2wHfRStqY@pks.im>
In-Reply-To: <20260320115211.177351-3-adrian.ratiu@collabora.com>

On Fri, Mar 20, 2026 at 01:52:03PM +0200, Adrian Ratiu wrote:
> Fix some minor style nits pointed by Patrick, Junio and Eric:

Tiny nit, not worth rerolling over: "pointed out by"

> diff --git a/builtin/hook.c b/builtin/hook.c
> index 83020dfb4f..e641614b84 100644
> --- a/builtin/hook.c
> +++ b/builtin/hook.c
> @@ -5,8 +5,6 @@
>  #include "gettext.h"
>  #include "hook.h"
>  #include "parse-options.h"
> -#include "strvec.h"
> -#include "abspath.h"

Another thing we could address while at it is to sort the headers
(except "builtin.h" of course). Feel free to ignore though.

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index e34edff406..991d6ca7d5 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -904,7 +904,8 @@ static int feed_receive_hook_cb(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_
>  static void *receive_hook_feed_state_alloc(void *feed_pipe_ctx)
>  {
>  	struct receive_hook_feed_state *init_state = feed_pipe_ctx;
> -	struct receive_hook_feed_state *data = xcalloc(1, sizeof(*data));
> +	struct receive_hook_feed_state *data;
> +	CALLOC_ARRAY(data, 1);

I think it might help the reader to have an empty line between variables
and logic.

> @@ -928,7 +929,11 @@ static int run_receive_hook(struct command *commands,
>  {
>  	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
>  	struct command *iter = commands;
> -	struct receive_hook_feed_state feed_init_state = { 0 };
> +	struct receive_hook_feed_state feed_init_state = {
> +		.cmd = commands,
> +		.skip_broken = skip_broken,
> +		.buf = STRBUF_INIT,
> +	};

Interesting. The buffer here isn't only a style fix, but an actual bug
fix, isn't it?

> diff --git a/hook.c b/hook.c
> index 67cc9a66df..349db729f6 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -227,7 +227,8 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
>  	/* Construct the cache from parsed configs. */
>  	strmap_for_each_entry(&cb_data.event_hooks, &iter, e) {
>  		struct string_list *hook_names = e->value;
> -		struct string_list *hooks = xcalloc(1, sizeof(*hooks));
> +		struct string_list *hooks;
> +		CALLOC_ARRAY(hooks, 1);
>  
>  		string_list_init_dup(hooks);
>  

Same nit here: I'd move the empty line to come before `CALLOC_ARRAY()`.

> @@ -311,7 +312,8 @@ static void list_hooks_add_configured(struct repository *r,
>  	for (size_t i = 0; configured_hooks && i < configured_hooks->nr; i++) {
>  		const char *friendly_name = configured_hooks->items[i].string;
>  		const char *command = configured_hooks->items[i].util;
> -		struct hook *hook = xcalloc(1, sizeof(struct hook));
> +		struct hook *hook;
> +		CALLOC_ARRAY(hook, 1);
>  
>  		if (options && options->feed_pipe_cb_data_alloc)
>  			hook->feed_pipe_cb_data =

And here.

None of my nits are really important, so please feel free to address or
ignore them as you like.

Patrick