Stefan Beller <sbel...@google.com> writes: > +static int next_byte(const char **cp, const char **endp, > + const struct diff_options *diffopt) > +{ > + int retval; > + > + if (*cp > *endp) > + return -1; > + > + if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE_CHANGE)) { > + while (*cp < *endp && isspace(**cp)) > + (*cp)++; > + /* > + * After skipping a couple of whitespaces, we still have to > + * account for one space. > + */ > + return (int)' ';
Do we need a cast here? > +static unsigned get_string_hash(struct emitted_diff_symbol *es, struct > diff_options *o) > +{ > + if (o->xdl_opts & XDF_WHITESPACE_FLAGS) { > + static struct strbuf sb = STRBUF_INIT; > + const char *ap = es->line, *ae = es->line + es->len; > + int c; > + > + strbuf_reset(&sb); > + while (ae > ap && isspace(*ae)) > + ae--; Not testing for the AT_EOL option here? It does not make a difference in correctness; two lines that differ only by their trailing whitespaces will be hashed into the same bin even when we are not using ignore-whitespace-at-eol, making the hashmap a bit less efficient than necessary. By the way, this is an unrelated tangent because I think you inherited this pattern by copying and pasting from elsewhere, but I think it would be better if we avoid casting the function pointer type like this: > + if (o->color_moved) { > + struct hashmap add_lines, del_lines; > + > + hashmap_init(&del_lines, > + (hashmap_cmp_fn)moved_entry_cmp, o, 0); > + hashmap_init(&add_lines, > + (hashmap_cmp_fn)moved_entry_cmp, o, 0); When hashmap_cmp_fn's definition changes, these two calling sites won't be caught as passing a incorrectly typed callback function by the compiler. Instead, we can match the actual implementation of the callback function, e.g. > +static int moved_entry_cmp(const struct diff_options *diffopt, > + const struct moved_entry *a, > + const struct moved_entry *b, > + const void *keydata) > +{ to the expected function type, i.e. static int moved_entry_cmp(const void *fndata, const void *entry, const void *entry_or_key, const void *keydata) { const struct diff_options *diffopt = fndata; const struct moved_entry *a = entry; const struct moved_entry *b = entry_or_key; ... by casting the parameters.