Subject: Re: [PATCH v3] diff: check range before dereferencing an array element

From: Junio C Hamano <[email-masked]>
Date: Tue, 29 Apr 2025 16:46:41 -0700

Jeff King <[email-masked]> writes:

> Ah, you're right. I was reading the offset check as "are we past the end
> of string" (guided by CodeQL's complaint), and if that were the case the
> logic would apply equally to all values we are checking.
>
> But that is not what is going on at all. The offset check is for "len -
> 1", and so is "do not do this one CR match for the final character of
> the string". And thus applying it elsewhere is wrong.
>
> And CodeQL's false positive is doubly wrong. We do not even need to say
> "the string is NUL-terminated, so it is OK in this case to look past the
> end-of-string". The check is not even a string bounds check at all.

Exactly.

That is what makes it hard to give a reasonable explanation in the
log message, which I thought that Dscho did a much better job in
this iteration.

Thanks.