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

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 30, 2017, 19:50 UTC
Message-ID
<xmqqefu1qmpy.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20170630191407.5381-3-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
Show 15 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.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  patch-ids.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
Nicely explained.  Thanks for polishing the series.
Show 36 quoted lines
>
> diff --git a/patch-ids.c b/patch-ids.c
> index 815c115811..b4166b0f38 100644
> --- a/patch-ids.c
> +++ b/patch-ids.c
> @@ -35,10 +35,10 @@ int commit_patch_id(struct commit *commit, struct diff_options *options,
>   * the side of safety.  The actual value being negative does not have
>   * any significance; only that it is non-zero matters.
>   */
> -static int patch_id_cmp(const void *unused_cmp_data,
> +static int patch_id_cmp(struct diff_options *opt,
>  			struct patch_id *a,
>  			struct patch_id *b,
> -			struct diff_options *opt)
> +			const void *unused_keydata)
>  {
>  	if (is_null_oid(&a->patch_id) &&
>  	    commit_patch_id(a->commit, opt, &a->patch_id, 0))
> @@ -59,7 +59,7 @@ int init_patch_ids(struct patch_ids *ids)
>  	DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
>  	diff_setup_done(&ids->diffopts);
>  	hashmap_init(&ids->patches, (hashmap_cmp_fn)patch_id_cmp,
> -		     NULL, 256);
> +		     &ids->diffopts, 256);
>  	return 0;
>  }
>  
> @@ -95,7 +95,7 @@ struct patch_id *has_commit_patch_id(struct commit *commit,
>  	if (init_patch_id_entry(&patch, commit, ids))
>  		return NULL;
>  
> -	return hashmap_get(&ids->patches, &patch, &ids->diffopts);
> +	return hashmap_get(&ids->patches, &patch, NULL);
>  }
>  
>  struct patch_id *add_commit_patch_id(struct commit *commit,
Previous: Stefan BellerNext: Jeff King
Message 24 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.