Re: [PATCH v5 05/10] grep/icase: avoid kwsset when -F is specified

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 29, 2016, 06:23 UTC
Message-ID: <CAPig+cS0Aqjn7+E4bh-QjXpaP+wfRFFkE2CuiAPbcdNv7-2cGw@mail.gmail.com>
In-Reply-To: <1453982183-24124-6-git-send-email-pclouds@gmail.com>

On Thu, Jan 28, 2016 at 6:56 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:

Show 14 quoted lines

> Similar to the previous commit, we can't use kws on icase search
> outside ascii range. But we can't simply pass the pattern to
> regcomp/pcre like the previous commit because it may contain regex
> special characters, so we need to quote the regex first.
>
> To avoid misquote traps that could lead to undefined behavior, we
> always stick to basic regex engine in this case. We don't need fancy
> features for grepping a literal string anyway.
>
> basic_regex_quote_buf() assumes that if the pattern is in a multibyte
> encoding, ascii chars must be unambiguously encoded as single
> bytes. This is true at least for UTF-8. For others, let's wait until
> people yell up. Chances are nobody uses multibyte, non utf-8 charsets
> any more..

s/any more../anymore./

Show 13 quoted lines

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/grep.c b/grep.c
> @@ -397,6 +398,24 @@ static int is_fixed(const char *s, size_t len)
> +static void compile_fixed_regexp(struct grep_pat *p, struct grep_opt *opt)
> +{
> +       struct strbuf sb = STRBUF_INIT;
> +       int err;
> +
> +       basic_regex_quote_buf(&sb, p->pattern);
> +       err = regcomp(&p->regexp, sb.buf, opt->regflags & ~REG_EXTENDED);
> +       if (opt->debug)
> +               fprintf(stderr, "fixed%s\n", sb.buf);

Did you want a space or colon or something after "fixed" for human consumption? (I realize that the test case doesn't care.)

> +       strbuf_release(&sb);
> +       if (err) {
> +               char errbuf[1024];
> +               regerror(err, &p->regexp, errbuf, 1024);

I guess this was copy/pasted from compile_regexp(), but for this new code, perhaps: s/1024/sizeof(errbuf)/

> +               regfree(&p->regexp);
> +               compile_regexp_failed(p, errbuf);
> +       }
> +}