Subject: Re: [PATCH v4 08/19] worktree: fix a trivial leak in prune_worktrees()

From: Junio C Hamano <[email-masked]>
Date: Tue, 17 Jan 2023 22:28:28 -0800

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> We were leaking both the "struct strbuf" in prune_worktrees(), as well
> as the "path" we got from should_prune_worktree(). Since these were
> the only two uses of the "struct string_list" let's change it to a
> "DUP" and push these to it with "string_list_append_nodup()".
> ...
> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
> index d34d77f8934..ba8d929d189 100755
> --- a/t/t3203-branch-output.sh
> +++ b/t/t3203-branch-output.sh
> @@ -1,6 +1,8 @@
>  #!/bin/sh
>  
>  test_description='git branch display tests'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/lib-terminal.sh

This is wrong, isn't it?

t3203 uses --points-at, which populates filter.points_at by calling
parse_opt_object_name().  Various members of the ref-filter
structure is never freed (and there is no API helper function in
ref-filter subsystem).

Other tests that use --points-at (e.g. t6302 and t7004) are not
marked with "passes_sanitize_leak", and this one shouldn't be,
either.

With the following squashed in, the branch seems to pass, but I am
not sure which is lessor of the two evils.  From the point of view
of the code maintenance, UNLEAK() to mark this singleton variable is
far cleaner to deal with than selectively running the leak checks
with the "passes_sanitize_leak" mechanism (which always feels like a
losing whack-a-mole hack).

 builtin/branch.c | 1 +
 1 file changed, 1 insertion(+)

diff --git c/builtin/branch.c w/builtin/branch.c
index f63fd45edb..4fe7757670 100644
--- c/builtin/branch.c
+++ w/builtin/branch.c
@@ -742,6 +742,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (filter.abbrev == -1)
 		filter.abbrev = DEFAULT_ABBREV;
 	filter.ignore_case = icase;
+	UNLEAK(filter);
 
 	finalize_colopts(&colopts, -1);
 	if (filter.verbose) {