Re: [PATCH 3/5] wildmatch: remove NEGATE_CLASS and NEGATE_CLASS2 macros

[Masahiro Yamada <masahiroy@kernel.org>]

On Fri, Feb 10, 2023 at 10:14 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Fri, Feb 10 2023, Masahiro Yamada wrote:
>
> > The other glob patterns are hard-coded in dowild(). Do likewise.
> > [...]
> > diff --git a/wildmatch.c b/wildmatch.c
> > index a510b3fd23..93800b8eac 100644
> > --- a/wildmatch.c
> > +++ b/wildmatch.c
> > @@ -14,10 +14,6 @@
> >
> >  typedef unsigned char uchar;
> >
> > -/* What character marks an inverted character class? */
> > -#define NEGATE_CLASS '!'
> > -#define NEGATE_CLASS2        '^'
>
> Thanks, maybe these made sense in rsync's codebase, but...
>
> >  #define CC_EQ(class, len, litmatch) ((len) == sizeof (litmatch)-1 \
> >                                   && *(class) == *(litmatch) \
> >                                   && strncmp((char*)class, litmatch, len) == 0)
> > @@ -137,12 +133,8 @@ static int dowild(const uchar *p, const uchar *text, unsigned int flags)
> >                       return WM_ABORT_ALL;
> >               case '[':
> >                       p_ch = *++p;
>
> ...as the context shows we hardcode most of these tokens.
>
> > -#ifdef NEGATE_CLASS2
> > -                     if (p_ch == NEGATE_CLASS2)
> > -                             p_ch = NEGATE_CLASS;
> > -#endif
>
> Hrm, but isn't this a logic error? No it's not, because...
>
> >                       /* Assign literal 1/0 because of "matched" comparison. */
> > -                     negated = p_ch == NEGATE_CLASS ? 1 : 0;
> > +                     negated = p_ch == '!' || p_ch == '^' ? 1 : 0;
>
> ...you're refcatoring thise while at it.


Yes, this is a trivial refactoring.


Or, do you suggest splitting this into two patches,
verbatim replace + a small clean-up?

>
> Personally I'd prefer to just see this change without this, and it came
> as a surprise given the commit message.
>
> I find the pre-image easier to read (sans the macro check, which you
> should be removing).
>


-- 
Best Regards
Masahiro Yamada