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

From: Brandon Williams <bmwill@google.com>
Date: Sep 28, 2017, 00:41 UTC
Message-ID: <20170928004137.GD68699@google.com>
In-Reply-To: <20170927221910.164552-1-jonathantanmy@google.com>

On 09/27, Jonathan Tan wrote:

Show 6 quoted lines

> 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.

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.

-- 
Brandon Williams