diff mbox series

[16/20] builtin/merge.c: free "&buf" on "Your local changes..." error

Message ID patch-16.20-95d59b914d0-20221228T175512Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series leak fixes: various simple leak fixes | expand

Commit Message

Ævar Arnfjörð Bjarmason Dec. 28, 2022, 6 p.m. UTC
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(-)

Comments

René Scharfe Dec. 28, 2022, 10:13 p.m. UTC | #1
Am 28.12.22 um 19:00 schrieb Ævar Arnfjörð Bjarmason:
> 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.

>  			}
>
>  			/* 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
>
>
diff mbox series

Patch

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;
 			}
 
 			/* 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