Re: [PATCH] oidmap: ensure map is initialized
From: Junio C Hamano <hidden>
Date: 2017-12-27 18:02:09
Brandon Williams [off-list ref] 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 <redacted> ---
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.
quoted hunk
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); }