git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Stefan Beller <sbeller@google.com>
Date
Feb 9, 2017, 17:43 UTC
Message-ID
<CAGZ79kb-uNnfrvCUk9PbXcFwOJSmSwccuSaZ2H_u61engE+H3A@mail.gmail.com>
In-Reply-To
<4a21dba7-76ef-6aec-b326-c1046f3daad2@alum.mit.edu>
On Thu, Feb 9, 2017 at 9:40 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
Show 28 quoted lines
> 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.
Oh, I did not see that.

Thanks, Stefan

>
> Thanks for your review!
> Michael
>
Previous: Michael HaggertyNext: Jeff King
Message 9 of 28 in “Store submodules in a hash, not a linked list”
  1. 0/5 Store submodules in a hash, not a linked listMichael Haggerty, Feb 9, 2017
  2. 3/5 register_ref_store(): new functionMichael Haggerty, Feb 9, 2017
  3. Stefan BellerFeb 9, 2017
  4. Michael HaggertyFeb 9, 2017
  5. Junio C HamanoFeb 9, 2017
  6. 1/5 refs: store submodule ref stores in a hashmapMichael Haggerty, Feb 9, 2017
  7. Stefan BellerFeb 9, 2017
  8. Michael HaggertyFeb 9, 2017
  9. Stefan BellerFeb 9, 2017
  10. Jeff KingFeb 9, 2017
  11. Junio C HamanoFeb 9, 2017
  12. Jeff KingFeb 10, 2017
  13. Michael HaggertyFeb 10, 2017
  14. 4/5 files_ref_store::submodule: use NULL for the main repositoryMichael Haggerty, Feb 9, 2017
  15. Stefan BellerFeb 9, 2017
  16. Junio C HamanoFeb 9, 2017
  17. Stefan BellerFeb 9, 2017
  18. 5/5 read_loose_refs(): read refs using resolve_ref_recursively()Michael Haggerty, Feb 9, 2017
  19. Stefan BellerFeb 9, 2017
  20. 2/5 refs: push the submodule attribute downMichael Haggerty, Feb 9, 2017
  21. Stefan BellerFeb 9, 2017
  22. Junio C HamanoFeb 9, 2017
  23. Junio C HamanoFeb 9, 2017
  24. Jeff KingFeb 9, 2017
  25. Michael HaggertyFeb 9, 2017
  26. Jeff KingFeb 10, 2017
  27. Michael HaggertyFeb 10, 2017
  28. Duy NguyenFeb 13, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.