On Fri, Jun 30, 2017 at 10:54 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> +static int next_byte(const char **cp, const char **endp,
>> + const struct diff_options *diffopt)
>> +{
>> + int retval;
>> +
>> + if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE_AT_EOL)) {
>> + while (*endp > *cp && isspace(**endp))
>> + (*endp)--;
>> + }
>
> This should be done by the callers (both moved_entry_cmp() and
> get_string_hash()) before starting to iterate over the bytes from
> the beginning, no?
Good point.
>> +
>> + retval = **cp;
>
> The char could be signed, and byte 0xff may become indistinguishable
> from the EOF (i.e. -1) you returned earlier.
Ah, I messed up there. I think EOF is wrong, too.
So maybe we'll just return 256 to indicate the end of memory chunk
to not have to deal with signedness
>> + if (ca != cb)
>> + return 1; /* differs */
>> + if (!ca)
>
> Shouldn't this check for "ca == -1", as we are not dealing with NUL
> terminated string but a <ptr, len> thing?
Yes, we'd check for the ending symbol instead of 0.
