On Thu, Jul 25, 2019 at 1:12 PM Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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. Elijah
