Re: [PATCH 1/5] refs: store submodule ref stores in a hashmap

[Michael Haggerty <mhagger@alum.mit.edu>]

On 02/09/2017 05:58 PM, Stefan Beller wrote:
>> @@ -1402,17 +1435,17 @@ struct ref_store *ref_store_init(const char *submodule)
>>
>>  struct ref_store *lookup_ref_store(const char *submodule)
>>  {
> 
>> +       if (!submodule_ref_stores.tablesize)
>> +               hashmap_init(&submodule_ref_stores, submodule_hash_cmp, 20);
> 
> 
> So we can lookup a submodule even before we initialized the subsystem?
> Does that actually happen? (It sounds like a bug to me.)
>
> Instead of initializing, you could return NULL directly here.

The lines you quoted are only concerned with bringing the (empty)
hashmap into existence if it hasn't been initialized already. (There's
no HASHMAP_INIT.) I don't know what you mean by "initialize the
subsystem". The only way to bring a ref_store *object* into existence is
currently to call get_ref_store(submodule), which calls
lookup_ref_store(submodule) to see if it already exists, and if not
calls ref_store_init(submodule) to instantiate it and register it in the
hashmap. There's nothing else that has to be initialize before that
(except maybe the usual startup config reading etc.)

I suppose this code path could be changed to return NULL without
initializing the hashmap, but the hashmap will be initialized a moment
later by ref_store_init(), so I don't see much difference either way.

Thanks for your review!
Michael