Skip to content

Commit beaa1d9

Browse files
peffgitster
authored andcommitted
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 939af16 (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 2bbedde commit beaa1d9

File tree

2 files changed

+13
-8
lines changed

2 files changed

+13
-8
lines changed

compat/terminal.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -479,10 +479,13 @@ struct escape_sequence_entry {
479479
};
480480

481481
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,
484484
const void *keydata)
485485
{
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);
486489
return strcmp(e1->sequence, keydata ? keydata : e2->sequence);
487490
}
488491

@@ -496,8 +499,7 @@ static int is_known_escape_sequence(const char *sequence)
496499
struct strbuf buf = STRBUF_INIT;
497500
char *p, *eol;
498501

499-
hashmap_init(&sequences, (hashmap_cmp_fn)sequence_entry_cmp,
500-
NULL, 0);
502+
hashmap_init(&sequences, sequence_entry_cmp, NULL, 0);
501503

502504
strvec_pushl(&cp.args, "infocmp", "-L", "-1", NULL);
503505
if (pipe_command(&cp, NULL, 0, &buf, 0, NULL, 0))

range-diff.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,16 +230,19 @@ static int read_patches(const char *range, struct string_list *list,
230230
}
231231

232232
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)
236236
{
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);
237240
return strcmp(a->diff, keydata ? keydata : b->diff);
238241
}
239242

240243
static void find_exact_matches(struct string_list *a, struct string_list *b)
241244
{
242-
struct hashmap map = HASHMAP_INIT((hashmap_cmp_fn)patch_util_cmp, NULL);
245+
struct hashmap map = HASHMAP_INIT(patch_util_cmp, NULL);
243246
int i;
244247

245248
/* First, add the patches of a to a hash map */

0 commit comments

Comments
 (0)