Re: [PATCH v2 05/18] maintenance: add commit-graph task

From: Jeff King <peff@peff.net>
Date: Jul 29, 2020, 00:22 UTC
Message-ID: <20200729002231.GA1636139@coredump.intra.peff.net>
In-Reply-To: <04552b1d2ed751a11eb7c50f6898cbc078b552b4.1595527000.git.gitgitgadget@gmail.com>

On Thu, Jul 23, 2020 at 05:56:27PM +0000, Derrick Stolee via GitGitGadget wrote:

Show 16 quoted lines

> +static int run_write_commit_graph(void)
> +{
> +	int result;
> +	struct argv_array cmd = ARGV_ARRAY_INIT;
> +
> +	argv_array_pushl(&cmd, "commit-graph", "write",
> +			 "--split", "--reachable", NULL);
> +
> +	if (opts.quiet)
> +		argv_array_push(&cmd, "--no-progress");
> +
> +	result = run_command_v_opt(cmd.argv, RUN_GIT_CMD);
> +	argv_array_clear(&cmd);
> +
> +	return result;
> +}

This is a pretty minor nit, but since I happened to be looking at the merge of all of the recent argv_array callers today... :)

You can write this a bit more succinctly by reusing the argv_array provided by the child_process:

  struct child_process cmd = CHILD_PROCESS_INIT;

  cmd.git_cmd = 1;
  argv_array_pushl(&cmd.args, "commit-graph", "write",
                   "--split", "--reachable", NULL);

  if (opts.quiet)
          argv_array_push(&cmd.args, "--no-progress");

  return run_command(&cmd);

Then you don't have to worry about freeing the argv memory, because it's handled automatically.

Like I said, quite minor, but it looks like this pattern appears in a few places, so it might be worth tweaking. And it would still work with the "pushf" people recommended to avoid extra strbufs (I saw another one in fetch_remote(), too).

-Peff