Re: [PATCH 15/19] merge-recursive: split internal fields into a separate struct

> On Thu, Jul 25, 2019 at 1:12 PM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > Hi Elijah,
> >
> > On Thu, 25 Jul 2019, Elijah Newren wrote:
> >
> > > merge_options has several internal fields that should not be set or read
> > > by external callers.  This just complicates the API.  Move them into an
> > > opaque merge_options_internal struct that is defined only in
> > > merge-recursive.c and keep these out of merge-recursive.h.
> >
> > This method requires an extra `malloc()`/`free()` pair, leaving room for
> > leaks.
> >
> > I'd rather keep the definition of that struct in `merge-recursive.h`,
> > and make the `priv` field a full struct instead of a pointer. That would
> > save on that extra allocation.
>
> Yes, it has one extra malloc()/free() pair for the entire merge operation.  But:
>   * That's a drop in the bucket compared to the allocations already
> performed during a merge
>   * The allocation and free happen in merge_start() and
> merge_finalize().  Not much to keep track of, so not much room for
> leaks.
>   * I want what's currently in merge-recursive.h to be as simple as
> possible; I implemented your suggestion first, but it doesn't simplify
> as much as possible.
>
> But, more importantly:
>   * I want to write an alternative merge strategy providing drop-in
> replacement functions for merge_trees(), merge_recursive(), and
> merge_recursive_generic(). Defining merge_options_internal inside
> merge-recursive.h would mean that I have to have _exactly_ the same
> internal options in my implementation of merge-ort.c.  That doesn't
> make sense.