Re: [PATCH v3] diff: check range before dereferencing an array element
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 29, 2025, 22:37 UTC
- Message-ID
- <xmqqldri4ms0.fsf@gitster.g>
- In-Reply-To
- <20250429215847.GB36727@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 15 quoted lines
>> @@ -892,7 +892,7 @@ static void fill_es_indent_data(struct emitted_diff_symbol *es) >> >> /* skip any \v \f \r at start of indentation */ >> while (s[off] == '\f' || s[off] == '\v' || >> - (s[off] == '\r' && off < len - 1)) >> + (off < len - 1 && s[off] == '\r')) >> off++; > > ...since the same pattern exists for the other s[off] checks, is it > worth future-proofing this like: > > while (off < len - 1 && > (s[off] == '\f' || s[off] == '\v' || s[off] == '\r') > > ?
But doesn't it change the semantics?
s[off] == '\f', even if off is at the end of the string, i.e. (off == len - 1), must trigger the off++ increment.
On the other hand, CR that is the part of CRLF at the end of line is *not* treated like other funny whitespace control characters. This "is off not at the end of line, if so check CR" comparison is about that.