Re: [PATCH] This fixes a minor memory leak (detected by LeakSanitizer) in git merge.

>
> "Kevin Backhouse via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Kevin Backhouse <kevinbackhouse@github.com>
> >
> > To reproduce (with an ASAN build):
> >
> > ```
> > mkdir test
> > cd test
> > git init
> > echo x > x.txt
> > git add .
> > git commit -m "WIP"
> > git checkout -b dev
> > echo y > x.txt
> > git add .
> > git commit -m "WIP"
> > git checkout main
> > echo z > x.txt
> > git add .
> > git commit -m "WIP"
> > echo a > x.txt
> > git add .
> > git merge dev
> > ```
>
> We'd rather not to see the above in the proposed log message; can't
> we add (a variation of) it to our test suite?
>
> > The fix is to call free_commit_list(merge_bases) when an error occurs.
>
> We usually have the description of what the problem is and give an
> analysis on why/how it happens, before presenting a solution.  Write
> it more like:
>
>     The caller of merge_ort_recursive() expects the commit list
>     passed in as the merge_bases parameter to be fully consumed by
>     the function and does not free it when the function returns.  In
>     normal cases, the commit list does get consumed, but when the
>     function returns early upon encountering an error, it forgets to
>     clean it up.
>
>     Fix this by freeing the list in the code paths for error returns.
>
> >  merge-ort-wrappers.c | 4 +++-
> >  merge-ort.c          | 4 +++-
>
> These two places and their fixes seem OK, but I have to wonder if
> these are complete fixes.
>
> > diff --git a/merge-ort-wrappers.c b/merge-ort-wrappers.c
> > index 4acedf3c338..aeb56c9970c 100644
> > --- a/merge-ort-wrappers.c
> > +++ b/merge-ort-wrappers.c
> > @@ -54,8 +54,10 @@ int merge_ort_recursive(struct merge_options *opt,
> >       struct tree *head = repo_get_commit_tree(opt->repo, side1);
> >       struct merge_result tmp;
> >
> > -     if (unclean(opt, head))
> > +     if (unclean(opt, head)) {
> > +             free_commit_list(merge_bases);
> >               return -1;
> > +     }
> >
> >       memset(&tmp, 0, sizeof(tmp));
> >       merge_incore_recursive(opt, merge_bases, side1, side2, &tmp);
>
> The function before this hunk appears to have very similar code
> structure.  Does it need the same fix, or if not why not?
>
> > diff --git a/merge-ort.c b/merge-ort.c
> > index 8631c997002..a0eb91fb011 100644
> > --- a/merge-ort.c
> > +++ b/merge-ort.c
> > @@ -5070,8 +5070,10 @@ static void merge_ort_internal(struct merge_options *opt,
> >               opt->branch1 = "Temporary merge branch 1";
> >               opt->branch2 = "Temporary merge branch 2";
> >               merge_ort_internal(opt, NULL, prev, next, result);
> > -             if (result->clean < 0)
> > +             if (result->clean < 0) {
> > +                     free_commit_list(merge_bases);
> >                       return;
> > +             }
>
> Before this function, there is a comment that this came from another
> function and it seems to still have a very similar code structure.
> Does the other function need the same fix, or if not why not?