Re: [PATCH 05/11] builtin/repack.c: avoid leaking child arguments
Derrick Stolee <stolee@gmail.com> writes:
Show 8 quoted lines
>> 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.
Show 11 quoted lines
>
>> - 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.
Junio C Hamano <gitster@pobox.com>