On Wed, Jan 19, 2022 at 6:31 PM Elijah Newren <newren@xxxxxxxxx> wrote:
>
> On Wed, Jan 19, 2022 at 7:53 AM Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx> wrote:
> >
> > On Thu, Dec 30 2021, Elijah Newren via GitGitGadget wrote:
> >
> > > From: Elijah Newren <newren@xxxxxxxxx>
> >
> > > +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.
Fix over here: https://lore.kernel.org/git/pull.1200.git.git.1642664835.gitgitgadget@xxxxxxxxx/T/
Has a couple bonus memory leak fixes too.
