From: Adrian Ratiu <adrian.ratiu@collabora.com>
Date: Tue, 16 Dec 2025 09:22:56 GMT
Subject: Re: [PATCH v4 10/11] receive-pack: convert update hooks to new API
Message-ID: <877bumhjbj.fsf@collabora.com>
In-Reply-To: <aUETkJZe_qCS6ZV0@pks.im>

On Tue, 16 Dec 2025, Patrick Steinhardt <ps@pks.im> wrote:
> On Thu, Dec 04, 2025 at 04:15:34PM +0200, Adrian Ratiu wrote:
>> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
>> index e8ee0e7321..d95df748cd 100644
>> --- a/builtin/receive-pack.c
>> +++ b/builtin/receive-pack.c
>> @@ -938,31 +938,26 @@ static int run_receive_hook(struct command *commands,
>>  	return status;
>>  }
>>  
>> -static int run_update_hook(struct command *cmd)
>> +static void hook_output_to_sideband(struct strbuf *output, void *cb_data UNUSED)
>>  {
>> -	struct child_process proc = CHILD_PROCESS_INIT;
>> -	int code;
>> -	const char *hook_path = find_hook(the_repository, "update");
>> -
>> -	if (!hook_path)
>> -		return 0;
>> +	if (output && output->len)
>> +		send_sideband(1, 2, output->buf, output->len, use_sideband);
>
> Nit, not worth a reroll: the buffer shouldn't ever be `NULL`, should it?
> Checking for `output->len` does make sense though, as we may receive
> empty buffers for keepalives.

Good point. NULL output should still be checked to avoid unexpected
segfaults but it should rather trigger a BUG() than a no-op.

Still pretty much harmless as is, will fix if I do a re-roll.