qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] checkpatch: Don't spuriously warn about /** com


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH] checkpatch: Don't spuriously warn about /** comment starters
Date: Fri, 18 Jan 2019 16:32:24 +0000

On Fri, 18 Jan 2019 at 16:26, Eric Blake <address@hidden> wrote:
>
> On 1/18/19 10:08 AM, Peter Maydell wrote:
>
> >>> The sequence "/\*\*?" was intended to match either "/*" or "/**",
> >>> but Perl's semantics for '?' allow it to backtrack and try the
> >>> "matches 0 chars" option if the "matches 1 char" choice leads to
> >>> a failure of the rest of the regex to match.  Switch to "/\*\*?+"
> >>> which uses what perlre(1) calls the "possessive" quantifier form:
> >>> this means that if it matches the "/**" string it will not later
> >>> backtrack to matching just the "/*" prefix.
> >>
> >> Just wondering if "/\*{1,2}" would also work (it may have to be spelled
> >> "/\*\{1,2}" - I never remember which flavors of regex have which
> >> extensions without rereading docs)
> >
> > Oh yes, that would probably be the less perl-specific way
> > to write it.
>
> Except it would probably fail in the same way as the non-greedy \*? -
> when \*{2} can't satisfy the regex, the engine will retry with \*{1} and
> then see the second * as noise.

OK, so the patch is good as it is, though it doesn't address
the possible incorrect warnings with trailing whitespace.

> > I'm not sure exactly what I was aiming for with that
> > part of the regex when I wrote it. The comment suggests
> > that I was looking for "non-blank", ie I didn't want to
> > fire on /* or /** followed by just trailing whitespace.
> > The regex as written is clearly not actually doing that,
> > though...
>
> Trailing whitespace is generally a problem anyways, though :)

Yes, but checkpatch already checks for that, so we don't want
to report an incorrect "wrong blockquote" warning as well as
the "trailing whitespace" error, which is what the current
code does:

ERROR: trailing whitespace
#159: FILE: hw/arm/mps2-tz.c:391:
+    /* $

WARNING: Block comments use a leading /* on a separate line
#159: FILE: hw/arm/mps2-tz.c:391:
+    /*

total: 1 errors, 1 warnings, 165 lines checked

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]