Re: [PATCH 0/3] grep: PCRE JIT fixes

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Jul 24 2019, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> There's a couple of patches fixing mistakes in the JIT code I added
>> for PCRE in <20190722181923.21572-1-dev+git@drbeat.li> and
>> <20190721194052.15440-1-carenas@gmail.com>
>>
>> This small series proposes to replace both of those. In both cases I
>> think we're better off just removing the relevant code. The commit
>> messages for the patches themselves make the case for that.
>
> I am not sure about the BUG() that practically never triggered so
> far (AFAICT, the check that guards the BUG() would trigger only if
> we later introduced a bug, calling the code to compile when we are
> not asked to do so)---wouldn't it be better to leave it in while
> there still are people who are touching the vicinity?

The BUG() in 1/3 is just checking if pcre2?_config() returns a boolean
when promised, so it amounts to black-box testing of that library.

I think code in that style is overly paranoid and verbose, it's
reasonable to just trust the library in that case.

I think the reason it ended up in the codebase in the first place was
converting some first-draft implementation I wrote where I was being
more paranoid about using the PCRE API as a black box.

> The other two I am perfectly OK with.  It is easy to resurrect the
> support for v1 (which may not even be needed for long) and resurrect
> the support for v2 with Carlo's fix, if it later turns out that some
> users may need to use a more complex pattern.
>
> Thanks.
>
>> Ævar Arnfjörð Bjarmason (3):
>>   grep: remove overly paranoid BUG(...) code
>>   grep: stop "using" a custom JIT stack with PCRE v2
>>   grep: stop using a custom JIT stack with PCRE v1
>>
>>  grep.c | 46 ++++++----------------------------------------
>>  grep.h |  9 ---------
>>  2 files changed, 6 insertions(+), 49 deletions(-)