Subject: Re: [[PATCH v2] 1/4] patch-ids: stop using a hand-rolled hashmap implementation

From: Junio C Hamano <[email-masked]>
Date: Fri, 29 Jul 2016 14:29:52 -0700

Kevin Willford <[email-masked]> writes:

> +static int patch_id_cmp(struct patch_id *a,
> +			struct patch_id *b,
> +			void *keydata)
>  {
> +	return hashcmp(a->patch_id, b->patch_id);
>  }
>  
>  int init_patch_ids(struct patch_ids *ids)
>  {
>  	memset(ids, 0, sizeof(*ids));
>  	diff_setup(&ids->diffopts);
>  	DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
>  	diff_setup_done(&ids->diffopts);
> +	hashmap_init(&ids->patches, (hashmap_cmp_fn)patch_id_cmp, 256);
>  	return 0;
>  }

This is a tangent, and I do not suggest to change patch 1/4 to flip
the style, but I am not sure if this is a good style, or casting it
the other way around is better from the type-checking point of view,
i.e.

    static int cmp_fn(const void *a_, const void *b_, const void *keydata)
    {
	struct patch_id *a = a_;
        struct patch_id *b = b_;
	return hashcmp(a->patch_id, b->patch_id);
    }

    ...
    	hashmap_init(..., cmp_fn, ...);
    ...

I see many existing calls to hashmap_init() follow this pattern, so
as I said, patch 1/4 is fine as-is.

Thanks.