On Fri, Feb 10, 2023 at 10:14 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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
