Re: [PATCH] oidmap: map with OID as key

On Wed, 27 Sep 2017 17:41:37 -0700
Brandon Williams [off-list ref] wrote:

On 09/27, Jonathan Tan wrote:

quoted

This is similar to using the hashmap in hashmap.c, but with an
easier-to-use API. In particular, custom entry comparisons no longer
need to be written, and lookups can be done without constructing a
temporary entry structure.

oidset has been updated to use oidmap.

After a quick glance at the code, I think it looks good.  I do have one
suggestion though.  This map is structured much like our internal
hashmap where if you want to include any data you have to implement your
own struct with the internal 'entry' struct as the first member in your
custom struct.  I personal dislike this method, despite the potential
memory savings, because it makes the data structure much more difficult
to use.

If all i wanted was a map from 'OID -> const char *' then I would need
to write all the boilerplate code to make a struct which contains the
map 'entry' struct + the 'const char *' entry.

You are also making the
caller responsible for allocating the individual entries instead of
letting the data structure take care of that internally.

In the case where your extra data ("const char *" in your example) fits
in a pointer, yes it's true that the "util" design eliminates the need
to define and allocate a struct. But if you need to store more than
that, you will still have that need.

To me it seems like a much simpler API for a map would be to just allow
callers to store a 'void *' as the value.

I agree that the API would be simpler.

My main motivation with this design is indeed to save memory, and not
inconvenience the user too much (in the case where you're storing things
larger than one pointer, you just need to remember to put the special
struct at the beginning of your struct), but if memory is not so
important, I agree that we can switch to the "util" design.