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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2423 bytes --]

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.


>>>               # Block comments use /* on a line of its own
>>>               if ($rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ &&      #inline /*...*/
>>> -                 $rawline =~ m@^\+.*/\*\*?[ \t]*.+[ \t]*$@) { # /* or /** non-blank
>>> +                 $rawline =~ m@^\+.*/\*\*?+[ \t]*.+[ \t]*$@) { # /* or /** non-blank
>>
>> Hmm - Isn't "[ \t]*.+[ \t]*$" the same as ".+$?"
> 
> (do you mean '$?"' at the end of your sentence, or '$" ?' ?)

Serves me right for adding "" after the fact.  You are correct, intended
punctuation should be ".+$"? (the 3-character regex, concluded by asking
a question if the 3-character form is the same as the 15-character
form), and where I should have gone further and asked if it was the same
as "." (since who cares whether there are more than one extra characters
between there and the end of the line - once we've found any character
at all, we've already found enough to flag the line).

> 
> 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 :)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]