git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Masahiro Yamada <masahiroy@kernel.org>
Date
Feb 10, 2023, 17:03 UTC
Message-ID
<CAK7LNASe4z4EQD6w++9Q7G9U2GEWeCgmANYUTWps2xs96ubzBg@mail.gmail.com>
In-Reply-To
<230210.865yc9wtkx.gmgdl@evledraar.gmail.com>

On Fri, Feb 10, 2023 at 10:14 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 42 quoted lines
>
>
> 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?

Show 7 quoted lines
>
> 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
Previous: Ævar Arnfjörð BjarmasonNext: Masahiro Yamada
Message 14 of 17 in “Clean up wildmatch.c”
  1. 0/5 Clean up wildmatch.cMasahiro Yamada, Feb 10, 2023
  2. 1/5 git-compat-util: add isblank() and isgraph()Masahiro Yamada, Feb 10, 2023
  3. Ævar Arnfjörð BjarmasonFeb 10, 2023
  4. Masahiro YamadaFeb 10, 2023
  5. Junio C HamanoFeb 10, 2023
  6. Masahiro YamadaFeb 10, 2023
  7. René ScharfeFeb 10, 2023
  8. Masahiro YamadaFeb 11, 2023
  9. René ScharfeFeb 11, 2023
  10. René ScharfeFeb 11, 2023
  11. 2/5 wildmatch: remove IS*() macrosMasahiro Yamada, Feb 10, 2023
  12. 3/5 wildmatch: remove NEGATE_CLASS and NEGATE_CLASS2 macrosMasahiro Yamada, Feb 10, 2023
  13. Ævar Arnfjörð BjarmasonFeb 10, 2023
  14. Masahiro YamadaFeb 10, 2023
  15. 5/5 wildmatch: more cleanups after killing ucharMasahiro Yamada, Feb 10, 2023
  16. 4/5 wildmatch: use char instead of ucharMasahiro Yamada, Feb 10, 2023
  17. Ævar Arnfjörð BjarmasonFeb 10, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.