git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v13 3/3] grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data

From
René Scharfe <l.s.r@web.de>
Date
Nov 18, 2021, 20:57 UTC
Message-ID
<c5223004-6db6-e808-f607-d11bd45e7d99@web.de>
In-Reply-To
<xmqqczmxxr8o.fsf@gitster.g>
Am 18.11.21 um 19:17 schrieb Junio C Hamano:
Show 28 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> Let's have a look at the map.  Here are the differences between the
>>> versions regarding use of PCRE2_UTF:
>>>
>>> o: opt->ignore_locale
>>> h: has_non_ascii(p->pattern)
>>> i: is_utf8_locale()
>>> l: !opt->ignore_case && (p->fixed || p->is_fixed)
>>>
>>> o h i l master hamza rene2
>>> 0 0 0 0      0     1     0
>>> 0 0 0 1      0     1     0
>>> 0 0 1 0      0     1     1
>>> 0 0 1 1      0     1     0  <== 7812.13, confirmed using fprint() debugging
>>>
>>> So http://public-inbox.org/git/0ea73e7a-6d43-e223-ab2e-24c684102856@web.de/
>>> should not have this breakage, because it doesn't enable PCRE2_UTF for
>>> literal patterns.
>>
>> PCRE2_UTF will also matter for literal patterns. Try to peel apart the
>> two bytes in "é" and match them under -i with/without PCRE_UTF.
>
> Sorry for being late to the party, but doesn't "literal" in the
> context of this thread mean the column "l" above, i.e. we are not
> ignoring case and fixed or is_fixed member is set?  So "under -i"
> disqualifies as an example for "will also matter for literal",
> doesn't it?
Correct.
Show 11 quoted lines
> In hindsight, I guess we could have pushed a bit harder when René's
>
> -	if (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&
> +	if (!opt->ignore_locale && is_utf8_locale() &&
>  	    !(!opt->ignore_case && (p->fixed || p->is_fixed)))
>  		options |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);
>
> in https://public-inbox.org/git/0ea73e7a-6d43-e223-ab2e-24c684102856@web.de/
> (is that what is called 'rene2' above?) was raised on Oct 17th to
> amend/fix Hamza's [v13 3/3]; that would have prevented 'master' from
> having this breakage?
Yes, that the change I meant with "rene2".
Show 20 quoted lines
> Carlo, in your [PATCH v2] in <20211117102329.95456-1-carenas@gmail.com>,
> I see that the #else side for older PCREv2 users essentially reverts
> what Hamza's [PATCH v13 3/3] did to this area.
>
> +#else
> +	if (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&
> +	    !(!opt->ignore_case && (p->fixed || p->is_fixed)))
> +		options |= PCRE2_UTF;
> +#endif
>
> I guess this is a lot of change in the amount of text involved but
> the least amount of actual change in the behaviour.  For those with
> newer PCREv2, the behaviour would be the same as v2.34.0, and for
> others, the behaviour would be the same as v2.33.0.
>
> Having said all that, because the consensus seems to be that the
> whole "when we should match in UTF mode" may need to be rethought, I
> think reverting Hamza's [v13 3/3] would be the simplest way to clean
> up the mess for v2.34.1 that will give us a cleaner slate to later
> build on, than applying this patch.

Makes sense to me. It gives a better starting point to solve the issue afresh without getting entangled in mind-melting boolean expressions.

René
Previous: Junio C HamanoNext: Ævar Arnfjörð Bjarmason
Message 37 of 47 in “grep: refactor next_match() and match_one_pattern() for external use”
  1. 1/3 grep: refactor next_match() and match_one_pattern() for external useHamza Mahfooz, Oct 15, 2021
  2. 3/3 grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 dataHamza Mahfooz, Oct 15, 2021
  3. Junio C HamanoOct 15, 2021
  4. René ScharfeOct 16, 2021
  5. Ævar Arnfjörð BjarmasonOct 16, 2021
  6. René ScharfeOct 16, 2021
  7. Junio C HamanoOct 17, 2021
  8. René ScharfeOct 17, 2021
  9. Ævar Arnfjörð BjarmasonOct 17, 2021
  10. Andreas SchwabNov 15, 2021
  11. Ævar Arnfjörð BjarmasonNov 15, 2021
  12. Carlo ArenasNov 16, 2021
  13. Andreas SchwabNov 16, 2021
  14. Carlo ArenasNov 16, 2021
  15. Andreas SchwabNov 16, 2021
  16. Andreas SchwabNov 16, 2021
  17. Carlo ArenasNov 16, 2021
  18. Andreas SchwabNov 16, 2021
  19. grep: avoid setting UTF mode when not neededCarlo Marcelo Arenas Belón, Nov 16, 2021
  20. Ævar Arnfjörð BjarmasonNov 16, 2021
  21. Hamza MahfoozNov 16, 2021
  22. Ævar Arnfjörð BjarmasonNov 17, 2021
  23. Carlo ArenasNov 16, 2021
  24. Junio C HamanoNov 16, 2021
  25. grep: avoid setting UTF mode when dangerous with PCRECarlo Marcelo Arenas Belón, Nov 17, 2021
  26. Junio C HamanoNov 18, 2021
  27. Carlo ArenasNov 18, 2021
  28. Hamza MahfoozNov 18, 2021
  29. Ævar Arnfjörð BjarmasonNov 19, 2021
  30. Hamza MahfoozNov 18, 2021
  31. René ScharfeNov 17, 2021
  32. Carlo ArenasNov 17, 2021
  33. Ævar Arnfjörð BjarmasonNov 17, 2021
  34. Carlo ArenasNov 17, 2021
  35. Ævar Arnfjörð BjarmasonNov 18, 2021
  36. Junio C HamanoNov 18, 2021
  37. René ScharfeNov 18, 2021
  38. Ævar Arnfjörð BjarmasonNov 19, 2021
  39. René ScharfeNov 19, 2021
  40. Carlo ArenasNov 19, 2021
  41. Junio C HamanoNov 19, 2021
  42. 2/3 pretty: colorize pattern matches in commit messagesHamza Mahfooz, Oct 15, 2021
  43. Junio C HamanoOct 15, 2021
  44. Hamza MahfoozOct 15, 2021
  45. Junio C HamanoOct 15, 2021
  46. Hamza MahfoozOct 15, 2021
  47. Junio C HamanoOct 15, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.