Re: [PATCH v3 1/9] show, log: provide a --remerge-diff capability

>
> On Wed, Jan 19, 2022 at 7:53 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> >
> > On Thu, Dec 30 2021, Elijah Newren via GitGitGadget wrote:
> >
> > > From: Elijah Newren <newren@gmail.com>
> >
> > > +static int do_remerge_diff(struct rev_info *opt,
> > > +                        struct commit_list *parents,
> > > +                        struct object_id *oid,
> > > +                        struct commit *commit)
> > > +{
> > > +     struct merge_options o;
> > > +     struct commit_list *bases;
> > > +     struct merge_result res = {0};
> > > +     struct pretty_print_context ctx = {0};
> > > +     struct commit *parent1 = parents->item;
> > > +     struct commit *parent2 = parents->next->item;
> > > +     struct strbuf parent1_desc = STRBUF_INIT;
> > > +     struct strbuf parent2_desc = STRBUF_INIT;
> > > +
> > > +     /* Setup merge options */
> > > +     init_merge_options(&o, the_repository);
> > > +     o.show_rename_progress = 0;
> > > +
> > > +     ctx.abbrev = DEFAULT_ABBREV;
> > > +     format_commit_message(parent1, "%h (%s)", &parent1_desc, &ctx);
> > > +     format_commit_message(parent2, "%h (%s)", &parent2_desc, &ctx);
> > > +     o.branch1 = parent1_desc.buf;
> > > +     o.branch2 = parent2_desc.buf;
> > > +
> > > +     /* Parse the relevant commits and get the merge bases */
> > > +     parse_commit_or_die(parent1);
> > > +     parse_commit_or_die(parent2);
> > > +     bases = get_merge_bases(parent1, parent2);
> >
> > There's existing leaks all over the place here unrelated to this new
> > code, so this is no big deal, but I noticed that get_merge_bases() here
> > leaks.
>
> Interesting.
>
> > Shouldn't it call free_commit_list() like e.g. diff_get_merge_base()
> > which invokes get_merge_bases() does on the return value?

> So, yes, I think there's a leak, but it's not due to this patch.  It's
> one that has been around since...the introduction of merge-recursive
> (though it originally computed the merge bases internally rather than
> allowing them to be passed in).  So, it's been around for quite a
> while.
>
> I'll look into it, and see if I can come up with a fix, but it doesn't
> really belong in this series.  I'll submit it separately.
>
> Thanks for the report.