Brandon Williams <bmwill@xxxxxxxxxx> writes:
> Ensure that an oidmap is initialized before attempting to add, remove,
> or retrieve an entry by simply performing the initialization step
> before accessing the underlying hashmap.
>
> Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx>
> ---
Looks sane. Thanks for illustrating the idea with actual code.
Essentially, you are using map->map.cmpfn as a boolean to see "have
we initialized this thing? if not, we need to initialize it on
demand".
By the way, I am somewhat more sympathetic than usual to Dscho's
"make oidmap_get() very aware of the internal implementation detail
of hashmap_get_from_hash() to micro-optimize by removing the check
from _get()". Such a layering violation is disgusting, and making
it deliberately shows an even worse design taste, but in this
particular case, because the oidmap API is too thin a layer on top
of hashmap, it is understandably a very tempting approach.
> oidmap.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/oidmap.c b/oidmap.c
> index 6db4fffcd..d9fb19ba6 100644
> --- a/oidmap.c
> +++ b/oidmap.c
> @@ -33,12 +33,19 @@ void oidmap_free(struct oidmap *map, int free_entries)
>
> void *oidmap_get(const struct oidmap *map, const struct object_id *key)
> {
> + if (!map->map.cmpfn)
> + return NULL;
> +
> return hashmap_get_from_hash(&map->map, hash(key), key);
> }
>
> void *oidmap_remove(struct oidmap *map, const struct object_id *key)
> {
> struct hashmap_entry entry;
> +
> + if (!map->map.cmpfn)
> + oidmap_init(map, 0);
> +
> hashmap_entry_init(&entry, hash(key));
> return hashmap_remove(&map->map, &entry, key);
> }
> @@ -46,6 +53,10 @@ void *oidmap_remove(struct oidmap *map, const struct object_id *key)
> void *oidmap_put(struct oidmap *map, void *entry)
> {
> struct oidmap_entry *to_put = entry;
> +
> + if (!map->map.cmpfn)
> + oidmap_init(map, 0);
> +
> hashmap_entry_init(&to_put->internal_entry, hash(&to_put->oid));
> return hashmap_put(&map->map, to_put);
> }
