hashmap: use expected signatures for comparison functions

We prefer for callback functions to match the signature with which they'll be called, rather than casting them to the correct type when assigning function pointers. Even though casting often works in the real world, it is a violation of the standard. We did a mass conversion in 939af16eac (hashmap_cmp_fn takes hashmap_entry params, 2019-10-06), but have grown a few new cases since then. Because of the cast, the compiler does not complain. However, as of clang-18, UBSan will catch these at run-time, and the case in range-diff.c triggers when running t3206. After seeing that one, I scanned the results of: git grep '_fn)[^(]' '*.c' | grep -v typedef and found a similar case in compat/terminal.c (which presumably isn't called in the test suite, since it doesn't trigger UBSan). There might be other cases lurking if the cast is done using a typedef that doesn't end in "_fn", but loosening it finds too many false positives. I also looked for: git grep ' = ([a-z_]*) *[a-z]' '*.c' to find assignments that cast, but nothing looked like a function. The resulting code is unfortunately a little longer, but the bonus of using container_of() is that we are no longer restricted to the hashmap_entry being at the start of the struct. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Aug 19, 2023 at 19:55 UTC beaa1d952b90523a167a5d3f24e0a8ce56a4afcd
2 files changed +13 -8
compat/terminal.c
+6 -4
@@ -479,10 +479,13 @@ struct escape_sequence_entry {
479 };
480
481 static int sequence_entry_cmp(const void *hashmap_cmp_fn_data UNUSED,
482 - const struct escape_sequence_entry *e1,
483 - const struct escape_sequence_entry *e2,
482 + const struct hashmap_entry *he1,
483 + const struct hashmap_entry *he2,
484 const void *keydata)
485 {
486 + const struct escape_sequence_entry
487 + *e1 = container_of(he1, const struct escape_sequence_entry, entry),
488 + *e2 = container_of(he2, const struct escape_sequence_entry, entry);
489 return strcmp(e1->sequence, keydata ? keydata : e2->sequence);
490 }
491
@@ -496,8 +499,7 @@ static int is_known_escape_sequence(const char *sequence)
499 struct strbuf buf = STRBUF_INIT;
500 char *p, *eol;
501
499 - hashmap_init(&sequences, (hashmap_cmp_fn)sequence_entry_cmp,
500 - NULL, 0);
502 + hashmap_init(&sequences, sequence_entry_cmp, NULL, 0);
503
504 strvec_pushl(&cp.args, "infocmp", "-L", "-1", NULL);
505 if (pipe_command(&cp, NULL, 0, &buf, 0, NULL, 0))
range-diff.c
+7 -4
@@ -230,16 +230,19 @@ cleanup:
230 }
231
232 static int patch_util_cmp(const void *cmp_data UNUSED,
233 - const struct patch_util *a,
234 - const struct patch_util *b,
235 - const char *keydata)
233 + const struct hashmap_entry *ha,
234 + const struct hashmap_entry *hb,
235 + const void *keydata)
236 {
237 + const struct patch_util
238 + *a = container_of(ha, const struct patch_util, e),
239 + *b = container_of(hb, const struct patch_util, e);
240 return strcmp(a->diff, keydata ? keydata : b->diff);
241 }
242
243 static void find_exact_matches(struct string_list *a, struct string_list *b)
244 {
242 - struct hashmap map = HASHMAP_INIT((hashmap_cmp_fn)patch_util_cmp, NULL);
245 + struct hashmap map = HASHMAP_INIT(patch_util_cmp, NULL);
246 int i;
247
248 /* First, add the patches of a to a hash map */