]> git.ipfire.org Git - thirdparty/git.git/commit
hashmap: use expected signatures for comparison functions
authorJeff King <peff@peff.net>
Sat, 19 Aug 2023 23:55:30 +0000 (19:55 -0400)
committerJunio C Hamano <gitster@pobox.com>
Sun, 20 Aug 2023 04:17:53 +0000 (21:17 -0700)
commitbeaa1d952b90523a167a5d3f24e0a8ce56a4afcd
treebe6d9eb76a827b2d0a129ecb1cf45793b0c5a51a
parent2bbeddee5d36dfa3fab16ccd0b78dcdcf66c3a99
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>
compat/terminal.c
range-diff.c