Re: [PATCH] diff.c: increment buffer pointer in all code path

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 19, 2017, 05:32 UTC
Message-ID: <xmqqvajbityj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqzi8niu1r.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 24 quoted lines

> Jeff King <peff@peff.net> writes:
>
>> it does. It just adjusts our "end pointer" to point to the last valid
>> character in the string (rather than one past), which seems to be the
>> convention that those loops (and next_byte) expect.
>
> Yeah I am not sure if I like this comparison at the beginning of the
> function:
>
>         static int next_byte(const char **cp, const char **endp,
>                              const struct diff_options *diffopt)
>         {
>                 int retval;
>
>                 if (*cp > *endp)
>                         return -1;
>
> but it says endp _is_ part of valid input, contrary to my intuition.
>
> And your change to the initialization of ae/be in moved_entry_cmp()
> makes it consistent with it, I think.
>
> But doesn't it mean ae computation in get_string_hash() also needs a
> massaging?

Ah, forget the last two lines. You do do the massaging in your patch.

My preference actually is to fix next_byte to follow the usual "when we end, it points one past the valid region", though.

Thanks for digging it through.