Re: [PATCH v2 6/9] builtin/rebase.c: fix "options.onto_name" leak

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Ævar

On 12/01/2023 12:45, Ævar Arnfjörð Bjarmason wrote:
> In [1] we started saving away the earlier xstrdup()'d
> "options.onto_name" assignment to free() it, but when [2] added this
> "keep_base" branch it didn't free() the already assigned value before
> re-assigning to "options.onto_name". Let's do that, and fix the memory
> leak.

As I said before I don't think this message makes any sense. It should 
just say that when --keep-base is given we need to free 
options.onto_name as it does not come from argv. As I also mentioned you 
do not need to add "free(to_free)" as it is unused at this point. 
Freeing it makes the reader wonder what was previously assigned to it.

Best Wishes

Phillip

> 1. 9dba809a69a (builtin rebase: support --root, 2018-09-04)
> 2. 414d924beb4 (rebase: teach rebase --keep-base, 2019-08-27)
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   builtin/rebase.c                 | 3 ++-
>   t/t3416-rebase-onto-threedots.sh | 1 +
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 0d8c050f6b3..b4857b89f19 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1660,7 +1660,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   		strbuf_addstr(&buf, options.upstream_name);
>   		strbuf_addstr(&buf, "...");
>   		strbuf_addstr(&buf, branch_name);
> -		options.onto_name = xstrdup(buf.buf);
> +		free(to_free);
> +		options.onto_name = to_free = xstrdup(buf.buf);
>   	} else if (!options.onto_name)
>   		options.onto_name = options.upstream_name;
>   	if (strstr(options.onto_name, "...")) {
> diff --git a/t/t3416-rebase-onto-threedots.sh b/t/t3416-rebase-onto-threedots.sh
> index ea501f2b42b..f8c4ed78c9e 100755
> --- a/t/t3416-rebase-onto-threedots.sh
> +++ b/t/t3416-rebase-onto-threedots.sh
> @@ -5,6 +5,7 @@ test_description='git rebase --onto A...B'
>   GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>   export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>   
> +TEST_PASSES_SANITIZE_LEAK=true
>   . ./test-lib.sh
>   . "$TEST_DIRECTORY/lib-rebase.sh"
>