Re: [PATCH 3/5] grep: remove redundant "fixed" field re-assignment to 0

From: Stefan Beller <sbeller@google.com>
Date: Jun 29, 2017, 17:10 UTC
Message-ID: <CAGZ79kaLEA+6DDgWnqw-hg1GrWVD7CgGNAkmXx_iQmeKGSJUqQ@mail.gmail.com>
In-Reply-To: <20170628215809.23060-4-avarab@gmail.com>

On Wed, Jun 28, 2017 at 2:58 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 24 quoted lines

> Remove the redundant re-assignment of the fixed field to zero right
> after the entire struct has been set to zero via memset(...).
>
> Unlike some nearby commits this pattern doesn't date back to the
> pattern described in e0b9f8ae09 ("grep: remove redundant regflags
> assignments", 2017-05-25), instead it was apparently cargo-culted in
> 9eceddeec6 ("Use kwset in grep", 2011-08-21).
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  grep.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/grep.c b/grep.c
> index 6614042fdc..7cd8a6512f 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -627,8 +627,6 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
>             has_null(p->pattern, p->patternlen) ||
>             is_fixed(p->pattern, p->patternlen))
>                 p->fixed = !icase || ascii_only;
> -       else
> -               p->fixed = 0;
>

I was about to propose a similar action as in 2/5, but getting the condition right is not as easy:

    p->fixed = (opt->fixed ||
           has_null(p->pattern, p->patternlen) ||
           is_fixed(p->pattern, p->patternlen)) &&
           (!icase || ascii_only);

does not look as convincing here.

Thanks for mentioning 9eceddeec6 as in that commit I would have been easy with just proposing to have

    p->fixed = opt->fixed || is_fixed(p->pattern, p->patternlen);

Thanks, Stefan