Re: [PATCH v2 8/8] grep: optimistically use PCRE2_MATCH_INVALID_UTF
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jul 26, 2019, 21:57 UTC
- Message-ID
- <877e84792k.fsf@evledraar.gmail.com>
- In-Reply-To
- <87a7d0799h.fsf@evledraar.gmail.com>
On Fri, Jul 26 2019, Ævar Arnfjörð Bjarmason wrote:
Show 71 quoted lines
> On Fri, Jul 26 2019, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>> diff --git a/Makefile b/Makefile
>>> index bd246f2989..dd38d5e527 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -726,6 +726,7 @@ TEST_BUILTINS_OBJS += test-oidmap.o
>>> TEST_BUILTINS_OBJS += test-online-cpus.o
>>> TEST_BUILTINS_OBJS += test-parse-options.o
>>> TEST_BUILTINS_OBJS += test-path-utils.o
>>> +TEST_BUILTINS_OBJS += test-pcre2-config.o
>>
>> This won't even build with any released pcre version; shouldn't we
>> make it at least conditionally compiled code? Specifically...
>>
>>> TEST_BUILTINS_OBJS += test-pkt-line.o
>>> TEST_BUILTINS_OBJS += test-prio-queue.o
>>> TEST_BUILTINS_OBJS += test-reach.o
>>> diff --git a/grep.c b/grep.c
>>> index c7c06ae08d..8b8b9efe12 100644
>>> --- a/grep.c
>>> +++ b/grep.c
>>> @@ -474,7 +474,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
>>> }
>>> if (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&
>>> !(!opt->ignore_case && (p->fixed || p->is_fixed)))
>>> - options |= PCRE2_UTF;
>>> + options |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);
>>>
>>> p->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,
>>> p->patternlen, options, &error, &erroffset,
>>> diff --git a/grep.h b/grep.h
>>> index c0c71eb4a9..506f05b97b 100644
>>> --- a/grep.h
>>> +++ b/grep.h
>>> @@ -21,6 +21,9 @@ typedef int pcre_extra;
>>> #ifdef USE_LIBPCRE2
>>> #define PCRE2_CODE_UNIT_WIDTH 8
>>> #include <pcre2.h>
>>> +#ifndef PCRE2_MATCH_INVALID_UTF
>>> +#define PCRE2_MATCH_INVALID_UTF 0
>>> +#endif
>>
>> ... unlike this piece of code ...
>>
>>> #else
>>> typedef int pcre2_code;
>>> typedef int pcre2_match_data;
>>> diff --git a/t/helper/test-pcre2-config.c b/t/helper/test-pcre2-config.c
>>> new file mode 100644
>>> index 0000000000..5258fdddba
>>> --- /dev/null
>>> +++ b/t/helper/test-pcre2-config.c
>>> @@ -0,0 +1,12 @@
>>> +#include "test-tool.h"
>>> +#include "cache.h"
>>> +#include "grep.h"
>>> +
>>> +int cmd__pcre2_config(int argc, const char **argv)
>>> +{
>>> + if (argc == 2 && !strcmp(argv[1], "has-PCRE2_MATCH_INVALID_UTF")) {
>>> + int value = PCRE2_MATCH_INVALID_UTF;
>>
>> ... this part does not have any fallback definition.
>
> It works because we include grep.h, which'll define
> PCRE2_MATCH_INVALID_UTF=0 if pcre2.h doesn't give it to us. I've tested
> this on PCRE versions with/without PCRE2_MATCH_INVALID_UTF and it works
> & runs/skips the appropriate tests.Ah, I spoke too soon, of course that's all guarded by "are we using PCRE v2 in general?". I'll fix it...