Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax

[Peter Maydell]

On 10 August 2018 at 07:22, Markus Armbruster <address@hidden> wrote:
> Peter Maydell <address@hidden> writes:
>
>> We now require Linux-kernel-style multiline comments:
>>     /*
>>      * line one
>>      * line two
>>      */
>>
>> Enforce this in checkpatch.pl, by backporting the relevant
>> parts of the Linux kernel's checkpatch.pl. (The only changes
>> needed are that Linux's checkpatch.pl WARN() function takes
>> an extra argument that ours does not.)
>
> Really?  [*]

Yes; the parts that I have taken from checkpatch.pl
are only modified by adjusting the WARN() function calls.

>> The kernel's checkpatch does not enforce "leading /* on
>> a line of its own, so that part is unique to QEMU's checkpatch.
>>
>> Sample warning output:
>>
>> WARNING: Block comments use a leading /* on a separate line
>> #34: FILE: hw/intc/arm_gicv3_common.c:39:
>> +    /* Older versions of QEMU had a bug in the handling of state 
>> save/restore
>>
>> Signed-off-by: Peter Maydell <address@hidden>
>> ---
>> I'm still not used to the leeading-/*-on-it's-own style,
>> so having checkpatch catch my lapses is handy...
>
> Yes, please!
>
>> I used WARN level severity mostly because Linux does.
>> ---
>>  scripts/checkpatch.pl | 48 +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 42e1c50dd80..18bc3c59c85 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -1566,6 +1566,54 @@ sub process {
>>  # check we are in a valid C source file if not then ignore this hunk
>>               next if ($realfile !~ /\.(h|c|cpp)$/);
>>
>> +# Block comment styles
>> +
>> +             # Block comments use /* on a line of its own
>> +             if ($rawline !~ address@hidden/\*.*\*/[ \t]*$@ &&      #inline 
>> /*...*/
>> +                 $rawline =~ address@hidden/\*\*?[ \t]*.+[ \t]*$@) { # /* 
>> or /** non-blank
>> +                     WARN("Block comments use a leading /* on a separate 
>> line\n" . $herecurr);
>> +             }
>> +

This is the bit that is "unique to QEMU's checkpatch",
per the commit message.


> [*] The kernel's has
>
>    # Networking with an initial /*
>                    if ($realfile =~ address@hidden(drivers/net/|net/)@ &&
>                        $prevrawline =~ /^\+[ \t]*\/\*[ \t]*$/ &&
>                        $rawline =~ /^\+[ \t]*\*/ &&
>                        $realline > 2) {
>                            WARN("NETWORKING_BLOCK_COMMENT_STYLE",
>                                 "networking block comments don't use an empty 
> /* line, use /* Comment...\n" . $hereprev);
>                    }
>
> which makes no sense for us.

This is a part which I did not take from kernel checkpatch;
it is not a "relevant part", per the commit message.

> With a corrected commit message:

The commit message still makes sense to me.

> Reviewed-by: Markus Armbruster <address@hidden>

thanks
-- PMM