From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 21 Oct 2021 18:47:11 GMT
Subject: Re: [PATCH 05/11] builtin/repack.c: avoid leaking child arguments
Message-ID: <xmqqh7dakyfk.fsf@gitster.g>
In-Reply-To: <67b553b7-4e69-9906-b237-9c44cf8aaf44@gmail.com>

Derrick Stolee <stolee@gmail.com> writes:

>>  	struct child_process cmd = CHILD_PROCESS_INIT;
>>  	FILE *out;
>>  	struct strbuf line = STRBUF_INIT;
>> +	int ret = 0;
>
> nit: "ret" is short for "return" which makes me think "this will
> be used as 'return ret;'" but we don't do that. Instead, we use
> it as the _result_ of an inner call:

Yup, my reading hiccupped there, too.

>
>> -	if (finish_command(&cmd))
>> +	ret = finish_command(&cmd);
>> +
>> +cleanup:
>> +	child_process_clear(&cmd);
>> +
>> +	if (ret)
>>  		die(_("could not finish pack-objects to repack promisor objects"));
>
> For that reason, I would rename this to "res" or "result".

Yeah, if we are introducing a new variable, result is a good name.