Stefan Beller <[email protected]> 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.