Re: [PATCH v2 03/10] hashmap: allow re-use after hashmap_free()

From: Jeff King <peff@peff.net>
Date: Nov 3, 2020, 16:08 UTC
Message-ID: <20201103160818.GB444466@coredump.intra.peff.net>
In-Reply-To: <CABPp-BHRSottxaYO+Ot5tPpNCf4scxmtwBLZicMJ4ChNEhmGHA@mail.gmail.com>

On Fri, Oct 30, 2020 at 08:37:42AM -0700, Elijah Newren wrote:

Show 10 quoted lines

> > This part I disagree with. If we did:
> >
> >   #define HASHMAP_INIT(fn, data) = { .cmpfn = cmpfn, cmpfn_data = data }
> >
> > then many callers could avoid handling the lazy-init themselves. E.g.:
> 
> Ah, gotcha.  That makes sense to me.  Given that 43 out of 47 callers
> of hashmap_init use cmpfn_data = NULL, should I shorten it to just one
> parameter for the macro, and let the four special cases keep calling
> hashmap_init() to specify a non-NULL cmpfn_data?

I'd be fine with it either way. I actually wrote it without the data parameter at first, then changed my mine and added it in. ;)

You could also do:

  #define HASHMAP_INIT_DATA(fn, data) { .cmpfn = cmpfn, cmpfn_data = data }
  #define HASHMAP_INIT(fn) HASHMAP_INIT_DATA(fn, NULL)

if you want to keep most callers simple.

-Peff