From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 04 Feb 2021 17:56:17 GMT
Subject: Re: [PATCH 14/25] pickaxe -S: remove redundant "sz" check in while-loop
Message-ID: <xmqqh7mr90ou.fsf@gitster.c.googlers.com>
In-Reply-To: <4ef09db7-34f2-2fb5-b9e9-be69c7102787@web.de>

René Scharfe <l.s.r@web.de> writes:

>> -		while (sz &&
>> -		       !regexec_buf(regexp, data, sz, 1, &regmatch, flags)) {
>> +		while (!regexec_buf(regexp, data, sz, 1, &regmatch, flags)) {
>
> This will loop forever for regexes that match an empty string.  An
> example would be /$/.  Silly, perhaps, but still I understand this check
> less as an optimization and more as a correctness/robustness thing.
>
>>  			flags |= REG_NOTBOL;
>>  			data += regmatch.rm_eo;
>>  			sz -= regmatch.rm_eo;
>> -			if (sz && regmatch.rm_so == regmatch.rm_eo) {
>> +			if (regmatch.rm_so == regmatch.rm_eo) {
>>  				data++;
>>  				sz--;
>>  			}
>
> Before, if the match was an empty string and there was more data after
> it, then the code would consume a character anyway, in order to avoid
> matching the same empty string again.  With the patch, that character
> is consumed even if there is no more data.  This leaves 'data'
> pointing beyond the buffer and 'sz' rolls over to ULONG_MAX.  Oops. :(

While I do not care too much about NUL in the haystack, I do not
mind [13/25] either.  But this is bad.

This whole thing reminds me of f53c5de2 (pickaxe: fix segfault with
'-S<...> --pickaxe-regex', 2017-03-18), by the way.

Thanks.