Re: [PATCHv2 1/2] hashmap.h: compare function has access to a data field

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 30, 2017, 18:40 UTC
Message-ID: <xmqqzicpqpyh.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kbWXuC2y97+nOsgvNhCCqg=2+VgOOvTbUn1LGpH4f0=PQ@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 21 quoted lines

> I am at a loss here after re-reading your answer over and over,
> but I think you are asking if patch_id_cmp can break, as
> we have a callchain like
>
>   patch_id_cmp
>     commit_patch_id
>       (diff_root_tree_oid)
>         diff_tree_oid
>           ll_diff_tree_oid
>
> passing diff_options down there, and patch_id_cmp may have
> gotten NULL.
>
> The answer is no, it was safe. (by accident?)
>
> That is because we never use hashmap_get_next
> on the hashmap that uses patch_id_cmp as the compare
> function.
>
> hashmap_get_next is the only function that does not pass
> on a keydata, any other has valid caller provided keydata.

Ah, thanks for clarifying. I think I misread the earlier discussion. So unless somebody goes in to patch-ids.c and adds a call to do get_next, we won't see a breakage, and it is not worth to do a test-patch-ids.c that peeks into the hashmap patch-ids.c uses and does get_next() only to demonstrate a potential future breakage.

OK.