Taylor Blau <me@xxxxxxxxxxxx> writes:
> @@ -586,8 +588,10 @@ static int write_midx_included_packs(struct string_list *include,
> strvec_pushf(&cmd.args, "--refs-snapshot=%s", refs_snapshot);
>
> ret = start_command(&cmd);
> - if (ret)
> + if (ret) {
> + child_process_clear(&cmd);
> return ret;
> + }
This happens only when start_command() returns an error. But the
function always calls child_process_clear() before doing so.
So I am not sure if this hunk is needed. It didn't exist in v1, if
I recall correctly. Am I missing something obvious?
> @@ -608,9 +612,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
> struct pack_geometry *geometry = NULL;
> struct strbuf line = STRBUF_INIT;
> struct tempfile *refs_snapshot = NULL;
> - int i, ext, ret;
> + int i, ext, ret = 0;
> FILE *out;
> int show_progress = isatty(2);
> + int cmd_cleared = 0;
>
> /* variables to be filled by option parsing */
> int pack_everything = 0;
> @@ -794,7 +799,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>
> ret = start_command(&cmd);
> if (ret)
> - return ret;
> + goto cleanup;
Likewise, at this point, start_command() should have already cleared
cmd before it returned an error, no?
> @@ -818,8 +823,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
> }
> fclose(out);
> ret = finish_command(&cmd);
> + cmd_cleared = 1;
> if (ret)
> - return ret;
> + goto cleanup;
And cmd is also cleared after we pass this point. So perhaps after
the cleanup label, there is no need to call child_process_clear() at
all, no?
