Hi Elijah,
On Mon, 21 Feb 2022, Elijah Newren wrote:
> On Sun, Feb 20, 2022 at 1:03 AM René Scharfe <l.s.r@xxxxxx> wrote:
> >
> > Am 20.02.22 um 07:54 schrieb Elijah Newren via GitGitGadget:
> [...]
> > > + /*
> > > + * Get the merge bases, in reverse order; see comment above
> > > + * merge_incore_recursive in merge-ort.h
> > > + */
> > > + common = get_merge_bases(parent1, parent2);
> > > + if (!common)
> > > + die(_("refusing to merge unrelated histories"));
> > > + for (j = common; j; j = j->next)
> > > + commit_list_insert(j->item, &merge_bases);
> >
> > This loop creates a reversed copy of "common". You could use
> > reverse_commit_list() instead to do it in-place and avoid the
> > allocations. Only the copy, "merge_bases", is used below.
>
> Oh, good catch. I probably should have been aware of this since
> someone requested I move the reverse_commit_list() function from
> merge-recursive.c to commit.c as part of my merge-ort work, but looks
> like I forgot about it and copied this command snippet from
> builtin/merge.c instead. I have no excuse.
Ooops! I missed that the `reverse_commit_list()` function was moved to
`commit.c` by _you_, and had not been there all along (my fault, of
course, see 8918b0c9c26 (merge-recur: try to merge older merge bases
first, 2006-08-09)).
> However, I wonder if that means we could also apply this
> simplification to the code snippets in builtin/merge.c and sequencer.c
> that you can find with
> git grep commit_list_insert.*reversed
> ? Maybe #leftoverbits for that part?
Yes, that's a good idea. I summarized this left-over-bit in
https://github.com/gitgitgadget/git/issues/1156
Ciao,
Dscho
