Subject: Re: [PATCH 05/20] builtin/merge-tree: fix leaking `-X` strategy options

From: Junio C Hamano <[email-masked]>
Date: Tue, 20 Aug 2024 15:10:31 -0700

Patrick Steinhardt <[email-masked]> writes:

> The `-X` switch for git-merge-tree(1) will push each option into a local
> `xopts` vector that we then end up parsing. The vector never gets freed
> though, causing a memory leak. Plug it.

It is unfortunate that the ownership rules of the second parameter
to parse_merge_opt() lets the string borrowed by the merge_options
struct.  Otherwise we could have just cleared xopts immediately
after we are done with calling it.

The clean-up looks correct.  Will queue.
Thanks.

>
> Signed-off-by: Patrick Steinhardt <[email-masked]>
> ---
>  builtin/merge-tree.c             | 13 ++++++++++---
>  t/t4301-merge-tree-write-tree.sh |  1 +
>  2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> index 9bca9b5f33c..c00469ed3db 100644
> --- a/builtin/merge-tree.c
> +++ b/builtin/merge-tree.c
> @@ -533,6 +533,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
>  	int expected_remaining_argc;
>  	int original_argc;
>  	const char *merge_base = NULL;
> +	int ret;
>  
>  	const char * const merge_tree_usage[] = {
>  		N_("git merge-tree [--write-tree] [<options>] <branch1> <branch2>"),
> @@ -625,7 +626,9 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
>  			strbuf_list_free(split);
>  		}
>  		strbuf_release(&buf);
> -		return 0;
> +
> +		ret = 0;
> +		goto out;
>  	}
>  
>  	/* Figure out which mode to use */
> @@ -664,7 +667,11 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
>  
>  	/* Do the relevant type of merge */
>  	if (o.mode == MODE_REAL)
> -		return real_merge(&o, merge_base, argv[0], argv[1], prefix);
> +		ret = real_merge(&o, merge_base, argv[0], argv[1], prefix);
>  	else
> -		return trivial_merge(argv[0], argv[1], argv[2]);
> +		ret = trivial_merge(argv[0], argv[1], argv[2]);
> +
> +out:
> +	strvec_clear(&xopts);
> +	return ret;
>  }
> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
> index eea19907b55..37f1cd7364c 100755
> --- a/t/t4301-merge-tree-write-tree.sh
> +++ b/t/t4301-merge-tree-write-tree.sh
> @@ -2,6 +2,7 @@
>  
>  test_description='git merge-tree --write-tree'
>  
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>  
>  # This test is ort-specific