Re: [PATCH v3 04/26] submodule--helper: fix a leak in "clone_submodule"

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 21, 2022, 21:30 UTC
Message-ID: <xmqq35euf92d.fsf@gitster.g>
In-Reply-To: <patch-v3-04.26-26f0caf0386-20220721T191249Z-avarab@gmail.com>

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

Show 20 quoted lines

> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 7d5ee6a6261..1ddce8e19c1 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1658,14 +1658,15 @@ static int clone_submodule(const struct module_clone_data *clone_data,
>  	char *sm_alternate = NULL, *error_strategy = NULL;
>  	struct child_process cp = CHILD_PROCESS_INIT;
>  	const char *clone_data_path;
> +	char *to_free = NULL;
>  
>  	if (!is_absolute_path(clone_data->path)) {
>  		struct strbuf sb = STRBUF_INIT;
>  
>  		strbuf_addf(&sb, "%s/%s", get_git_work_tree(), clone_data->path);
> -		clone_data_path = strbuf_detach(&sb, NULL);
> +		clone_data_path = to_free = strbuf_detach(&sb, NULL);
>  	} else {
> -		clone_data_path = xstrdup(clone_data_path);
> +		clone_data_path = clone_data->path;
>  	}

Heh, the bug I noticed in the previous step is silently fixed here.

This is why I do not trust a series that is artificially split into steps and sent out without self reviewing or even compiling them. Forces reviewers to do more work wasting their time reviewing and finding bugs that ends up not mattering because that weren't even something the author intended to write in the end.