Re: [PATCH 3/5] run-command API users: use strvec_pushv(), not argv assignment

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 22, 2021, 21:19 UTC
Message-ID: <xmqqlf1fevm7.fsf@gitster.g>
In-Reply-To: <patch-3.5-fd6c4c960ba-20211122T153605Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 22 quoted lines

> diff --git a/add-patch.c b/add-patch.c
> index 8c41cdfe39b..573eef0cc4a 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -413,7 +413,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
>  		strvec_push(&args, ps->items[i].original);
>  
>  	setup_child_process(s, &cp, NULL);
> -	cp.argv = args.v;
> +	strvec_pushv(&cp.args, args.v);
>  	res = capture_command(&cp, plain, 0);
>  	if (res) {
>  		strvec_clear(&args);
> @@ -431,7 +431,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
>  
>  		setup_child_process(s, &colored_cp, NULL);
>  		xsnprintf((char *)args.v[color_arg_index], 8, "--color");
> -		colored_cp.argv = args.v;
> +		strvec_pushv(&colored_cp.args, args.v);
>  		colored = &s->colored;
>  		res = capture_command(&colored_cp, colored, 0);
>  		strvec_clear(&args);

We used to use the caller-supplied args, run-command API borrowed
that strvec by pointing at cp.argv, and because it is borrowed,
capture_command() did not use cp.args/cp.env_array and there was no
leak inside run-command API side, whether capture_command succeeded
or failed.  The code was using its own args, so it already correctly
releases it (we can see one such strvec_clear() here).
 
OK.

I gave the remainder only a cursory look so I cannot call it quite "reviewed", but presumably all the other changes in this patch are the same way?

Thanks.