From: Phillip Wood Date: Tue, 24 Jan 2023 14:40:05 GMT Subject: Re: [PATCH v3 5/8] builtin/rebase.c: fix "options.onto_name" leak Message-ID: <3c836a15-f614-36f0-b0a4-efa1c3a496a0@dunelm.org.uk> In-Reply-To: Hi Ævar On 18/01/2023 16:09, Ævar Arnfjörð Bjarmason wrote: > Similar to the existing "squash_onto_name" added in [1] we need to > free() the xstrdup()'d "options.onto.name" added for "--keep-base" in > [2].. Apart from the unnecessary free() I was quite happy with the previous implementation that renamed squash_onto_name and used that but this patch looks fine. 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 > --- > builtin/rebase.c | 4 +++- > t/t3416-rebase-onto-threedots.sh | 1 + > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 5859a5387d8..5c474fb6edd 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -1037,6 +1037,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > struct string_list strategy_options = STRING_LIST_INIT_NODUP; > struct object_id squash_onto; > char *squash_onto_name = NULL; > + char *keep_base_onto_name = NULL; > int reschedule_failed_exec = -1; > int allow_preemptive_ff = 1; > int preserve_merges_selected = 0; > @@ -1660,7 +1661,7 @@ 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); > + options.onto_name = keep_base_onto_name = xstrdup(buf.buf); > } else if (!options.onto_name) > options.onto_name = options.upstream_name; > if (strstr(options.onto_name, "...")) { > @@ -1836,6 +1837,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > free(options.strategy); > strbuf_release(&options.git_format_patch_opt); > free(squash_onto_name); > + free(keep_base_onto_name); > string_list_clear(&exec, 0); > string_list_clear(&strategy_options, 0); > return !!ret; > 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" >