Re: [PATCH v5 04/10] grep/icase: avoid kwsset on literal non-ascii strings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 29, 2016 at 1:18 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Thu, Jan 28, 2016 at 6:56 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
>> When we detect the pattern is just a literal string, we avoid heavy
>> regex engine and use fast substring search implemented in kwsset.c.
>> But kws uses git-ctype which is locale-independent so it does not know
>> how to fold case properly outside ascii range. Let regcomp or pcre
>> take care of this case instead. Slower, but accurate.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>> ---
>> diff --git a/grep.c b/grep.c
>> @@ -398,12 +399,16 @@ static int is_fixed(const char *s, size_t len)
>>  static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
>>  {
>> +       int icase_non_ascii;
>>         int err;
>>
>>         p->word_regexp = opt->word_regexp;
>>         p->ignore_case = opt->ignore_case;
>> +       icase_non_ascii =
>> +               (opt->regflags & REG_ICASE || p->ignore_case) &&
>> +               has_non_ascii(p->pattern);
>>
>> -       if (is_fixed(p->pattern, p->patternlen))
>> +       if (!icase_non_ascii && is_fixed(p->pattern, p->patternlen))
>
> These double negatives (!non_ascii) here and in patch 5/10 are
> difficult to grok. I wonder if a different name, such as
> icase_ascii_only would help.

By the way, this is such a minor issue, and since there are only two
cases (in patches 4/10 and 5/10), it's not worth worrying about, and
certainly not worth a reroll.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]