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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 29, 2016, 21:29 UTC
Message-ID: <xmqqeg6cm8tr.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160729161920.3792-2-kcwillford@gmail.com>

Kevin Willford <kcwillford@gmail.com> writes:

Show 16 quoted lines

> +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.