Re: [PATCH 0/3] grep: PCRE JIT fixes
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jul 24, 2019, 20:03 UTC
- Message-ID
- <87k1c76vyw.fsf@evledraar.gmail.com>
- In-Reply-To
- <xmqq1ryfs8ws.fsf@gitster-ct.c.googlers.com>
On Wed, Jul 24 2019, Junio C Hamano wrote:
Show 15 quoted lines
> Æ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.
Show 15 quoted lines
> 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(-)