Re: [PATCH 07/10] builtin/rebase.c: fix "options.onto_name" leak
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Dec 31, 2022, 14:59 UTC
- Message-ID
- <33fc8bda-eedc-d1bf-7192-1cf1df5f6b3d@dunelm.org.uk>
- In-Reply-To
- <patch-07.10-484ebbfd6d1-20221230T071741Z-avarab@gmail.com>
Hi Ævar
On 30/12/2022 07:28, Ævar Arnfjörð Bjarmason wrote:
Show 5 quoted lines
> 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 > "squash_onto_name" before re-assigning to "options.onto_name". Let's > do that, and fix the memory leak.
I'm afraid I don't really follow the reasoning here. "squash_onto_name" is only used if --root is given and that option is incompatible with --keep-base so the --keep-base branch has no business touching it. Having said that I think you've stumbled upon a viable solution if you drop the unneeded free() added below and rename "squash_onto_name" to something like "onto_name_to_free".
Best Wishes
Phillip
Show 35 quoted lines
> 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 91bf55be6e6..f7fd20d71c0 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1658,7 +1658,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(squash_onto_name);
> + options.onto_name = squash_onto_name = 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"
>