Derrick Stolee <stolee@xxxxxxxxx> 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.