Re: [PATCH 16/20] builtin/merge.c: free "&buf" on "Your local changes..." error
- From
René Scharfe <l.s.r@web.de>
- Date
- Dec 28, 2022, 22:13 UTC
- Message-ID
- <24cd9d7c-2eb5-e17b-3cd2-98a2c6bd1424@web.de>
- In-Reply-To
- <patch-16.20-95d59b914d0-20221228T175512Z-avarab@gmail.com>
Am 28.12.22 um 19:00 schrieb Ævar Arnfjörð Bjarmason:
Show 25 quoted lines
> Plug a memory leak introduced in [1], since that change didn't follow
> the "goto done" pattern introduced in [2] we'd leak the "&buf" memory.
>
> 1. e4cdfe84a0d (merge: abort if index does not match HEAD for trivial
> merges, 2022-07-23)
> 2. d5a35c114ab (Copy resolve_ref() return value for longer use,
> 2011-11-13)
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> builtin/merge.c | 3 ++-
> t/t6439-merge-co-error-msgs.sh | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 8f78f326dbe..e29b456f92c 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1623,7 +1623,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> error(_("Your local changes to the following files would be overwritten by merge:\n %s"),
> sb.buf);
> strbuf_release(&sb);
> - return 2;
> + ret = 2;
> + goto done;Good change -- only a single return remains, which is easier to handle.
If it was only about "buf" then moving its strbuf_release() call way up to free it immediately after its last use would work as well.
Show 15 quoted lines
> } > > /* See if it is really trivial. */ > diff --git a/t/t6439-merge-co-error-msgs.sh b/t/t6439-merge-co-error-msgs.sh > index 52cf0c87690..0cbec57cdab 100755 > --- a/t/t6439-merge-co-error-msgs.sh > +++ b/t/t6439-merge-co-error-msgs.sh > @@ -5,6 +5,7 @@ test_description='unpack-trees error messages' > GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main > export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > > +TEST_PASSES_SANITIZE_LEAK=true > . ./test-lib.sh > >