Re: [PATCH v2] bisect--helper: plug strvec leak

From: Jeff King <peff@peff.net>
Date: Oct 11, 2022, 02:39 UTC
Message-ID: <Y0TXTl0gSBOFQa9B@coredump.intra.peff.net>
In-Reply-To: <1965b54b-122a-c965-f886-1a7dd6afbfb4@web.de>

On Fri, Oct 07, 2022 at 05:08:42PM +0200, René Scharfe wrote:

Show 16 quoted lines

> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 501245fac9..28ef7ec2a4 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -765,11 +765,10 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, const char **a
>  		strbuf_read_file(&start_head, git_path_bisect_start(), 0);
>  		strbuf_trim(&start_head);
>  		if (!no_checkout) {
> -			struct strvec argv = STRVEC_INIT;
> +			const char *argv[] = { "checkout", start_head.buf,
> +					       "--", NULL };
> 
> -			strvec_pushl(&argv, "checkout", start_head.buf,
> -				     "--", NULL);
> -			if (run_command_v_opt(argv.v, RUN_GIT_CMD)) {
> +			if (run_command_v_opt(argv, RUN_GIT_CMD)) {

This is OK with me, but note that one thing we lose is compiler protection that we remembered the trailing NULL pointer in the argv array (whereas strvec_pushl() has an attribute that makes sure the last argument is a NULL).

Probably not that big a deal in practice. It would be nice if there was a way to annotate this for the compiler, but I don't think there's any attribute for "this pointer-to-pointer parameter should have a NULL at the end".

-Peff