Re: [PATCH 10/12] builtin/merge: free found_ref when done

[Elijah Newren <newren@gmail.com>]

On Sun, Jun 20, 2021 at 8:15 AM <andrzej@ahunt.org> wrote:
>
> From: Andrzej Hunt <ajrhunt@google.com>
>
> merge_name() calls dwim_ref(), which allocates a new string into
> found_ref. Therefore add a free() to avoid leaking found_ref.
>
> LSAN output from t0021:
>
> Direct leak of 16 byte(s) in 1 object(s) allocated from:
>     #0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
>     #1 0xa8beb8 in xstrdup wrapper.c:29:14
>     #2 0x954054 in expand_ref refs.c:671:12
>     #3 0x953cb6 in repo_dwim_ref refs.c:644:22
>     #4 0x5d3759 in dwim_ref refs.h:162:9
>     #5 0x5d3759 in merge_name builtin/merge.c:517:6
>     #6 0x5d3759 in collect_parents builtin/merge.c:1214:5
>     #7 0x5cf60d in cmd_merge builtin/merge.c:1458:16
>     #8 0x4ce83e in run_builtin git.c:475:11
>     #9 0x4ccafe in handle_builtin git.c:729:3
>     #10 0x4cb01c in run_argv git.c:818:4
>     #11 0x4cb01c in cmd_main git.c:949:19
>     #12 0x6bdbfd in main common-main.c:52:11
>     #13 0x7f0430502349 in __libc_start_main (/lib64/libc.so.6+0x24349)
>
> SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s).
>
> Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
> ---
>  builtin/merge.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index a8a843b1f5..7ad85c044a 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -503,7 +503,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
>         struct strbuf bname = STRBUF_INIT;
>         struct merge_remote_desc *desc;
>         const char *ptr;
> -       char *found_ref;
> +       char *found_ref = NULL;
>         int len, early;
>
>         strbuf_branchname(&bname, remote, 0);
> @@ -586,6 +586,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
>         strbuf_addf(msg, "%s\t\tcommit '%s'\n",
>                 oid_to_hex(&remote_head->object.oid), remote);
>  cleanup:
> +       free(found_ref);
>         strbuf_release(&buf);
>         strbuf_release(&bname);
>  }
> --
> 2.26.2

Makes sense, and a quick grep through the code doesn't suggest any
other obvious leaks from using dwim_ref().