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

Re: [PATCHv3 2/3] patch-ids.c: use hashmap correctly

From
Jeff King <peff@peff.net>
Date
Jul 5, 2017, 09:00 UTC
Message-ID
<20170705090003.ervwwantjliim3az@sigill.intra.peff.net>
In-Reply-To
<20170630191407.5381-3-sbeller@google.com>
On Fri, Jun 30, 2017 at 12:14:06PM -0700, Stefan Beller wrote:
Show 10 quoted lines
> As eluded to in the previous patch, the code in patch-ids.c is using
> the hashmaps API wrong.
> 
> Luckily we do not have a bug, as all hashmap functionality that we use
> here (hashmap_get) passes through the keydata.  If hashmap_get_next were
> to be used, a bug would occur as that passes NULL for the key_data.
> 
> So instead use the hashmap API correctly and provide the caller required
> data in the compare function via the first argument that always gets
> passed and was setup via the hashmap_init function.

Reviewing this a bit late, but it looks good to me. And I think the explanation above nicely covers what is going on (and why it isn't a bug).

-Peff
PS I think you meant "alluded" in the first sentence, unless you really
   were trying to escape the previous patch. :)
Previous: Junio C HamanoNext: Stefan Beller
Message 25 of 26 in “Introduce data field in hashmap and migrate docs to header”
  1. 0/2 Introduce data field in hashmap and migrate docs to headerStefan Beller, Jun 29, 2017
  2. 1/2 hashmap.h: compare function has access to a data fieldStefan Beller, Jun 29, 2017
  3. Junio C HamanoJun 29, 2017
  4. Junio C HamanoJun 29, 2017
  5. Stefan BellerJun 29, 2017
  6. Junio C HamanoJun 30, 2017
  7. 2/2 hashmap: migrate documentation from Documentation/technical into headerStefan Beller, Jun 29, 2017
  8. Jeff HostetlerJun 29, 2017
  9. Jonathan NiederJun 29, 2017
  10. Stefan BellerJun 29, 2017
  11. 0/2 Introduce data field in hashmap and migrate docs to headerStefan Beller, Jun 29, 2017
  12. 1/2 hashmap.h: compare function has access to a data fieldStefan Beller, Jun 29, 2017
  13. Junio C HamanoJun 30, 2017
  14. Stefan BellerJun 30, 2017
  15. Junio C HamanoJun 30, 2017
  16. Stefan BellerJun 30, 2017
  17. Junio C HamanoJun 30, 2017
  18. Stefan BellerJun 30, 2017
  19. Junio C HamanoJun 30, 2017
  20. 2/2 hashmap: migrate documentation from Documentation/technical into headerStefan Beller, Jun 29, 2017
  21. 0/3 Introduce data field in hashmap and migrate docs to headerStefan Beller, Jun 30, 2017
  22. 1/3 hashmap.h: compare function has access to a data fieldStefan Beller, Jun 30, 2017
  23. 2/3 patch-ids.c: use hashmap correctlyStefan Beller, Jun 30, 2017
  24. Junio C HamanoJun 30, 2017
  25. Jeff KingJul 5, 2017
  26. 3/3 hashmap: migrate documentation from Documentation/technical into headerStefan Beller, Jun 30, 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.