Subject: Re: [PATCH 05/11] builtin/repack.c: avoid leaking child arguments

From: Junio C Hamano <[email-masked]>
Date: Thu, 21 Oct 2021 11:47:11 -0700

Derrick Stolee <[email-masked]> 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.