On Fri, Feb 10, 2023 at 10:14 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

On Fri, Feb 10 2023, Masahiro Yamada wrote:

quoted

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...

quoted

 #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.

quoted

-#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...

quoted

                      /* 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