Re: [PATCH v3 13/19] builtin/merge.c: always free "struct strbuf msg"

From: René Scharfe <l.s.r@web.de>
Date: Jan 10, 2023, 18:52 UTC
Message-ID: <71e7d424-b785-9f9e-8b09-955b56fa19b0@web.de>
In-Reply-To: <patch-v3-13.19-6d99fdcc44e-20230110T054138Z-avarab@gmail.com>

Am 10.01.23 um 06:43 schrieb Ævar Arnfjörð Bjarmason:

> Follow-up 465028e0e25 (merge: add missing strbuf_release(),
> 2021-10-07) and address the "msg" memory leak in this block. We could
> free "&msg" before the "goto done" here, but even better is to avoid
> allocating it in the first place.

So contrary to the subject we don't need to free it anymore.

Show 60 quoted lines

>
> By repeating the "Fast-forward" string here we can avoid using a
> "struct strbuf" altogether.
>
> Suggested-by: René Scharfe <l.s.r@web.de>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/merge.c                | 11 ++++-------
>  t/t6439-merge-co-error-msgs.sh |  1 +
>  2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 0f093f2a4f2..91dd5435c59 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1560,7 +1560,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  			!common->next &&
>  			oideq(&common->item->object.oid, &head_commit->object.oid)) {
>  		/* Again the most common case of merging one remote. */
> -		struct strbuf msg = STRBUF_INIT;
> +		const char *msg = have_message ?
> +			"Fast-forward (no commit created; -m option ignored)" :
> +			"Fast-forward";
>  		struct commit *commit;
>
>  		if (verbosity >= 0) {
> @@ -1570,10 +1572,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  			       find_unique_abbrev(&remoteheads->item->object.oid,
>  						  DEFAULT_ABBREV));
>  		}
> -		strbuf_addstr(&msg, "Fast-forward");
> -		if (have_message)
> -			strbuf_addstr(&msg,
> -				" (no commit created; -m option ignored)");
>  		commit = remoteheads->item;
>  		if (!commit) {
>  			ret = 1;
> @@ -1592,9 +1590,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  			goto done;
>  		}
>
> -		finish(head_commit, remoteheads, &commit->object.oid, msg.buf);
> +		finish(head_commit, remoteheads, &commit->object.oid, msg);
>  		remove_merge_branch_state(the_repository);
> -		strbuf_release(&msg);
>  		goto done;
>  	} else if (!remoteheads->next && common->next)
>  		;
> 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
>
>