| Message ID | patch-07.10-484ebbfd6d1-20221230T071741Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | sequencer API & users: fix widespread leaks | expand |
Hi Ævar On 30/12/2022 07:28, Æ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 > "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 > 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" >
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"
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. 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(-)