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

[Markus Armbruster]

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?  [*]

>
> 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);
> +             }
> +

[*] 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.  Your code looks good to me, but your
commit message claims it doesn't exist :)

The remainder matches the kernel's version.

> +# Block comments use * on subsequent lines
> +             if ($prevline =~ /$;[ \t]*$/ &&                 #ends in comment
> +                 $prevrawline =~ /^\+.*?\/\*/ &&             #starting /*
> +                 $prevrawline !~ /\*\/[ \t]*$/ &&            #no trailing */
> +                 $rawline =~ /^\+/ &&                        #line is new
> +                 $rawline !~ /^\+[ \t]*\*/) {                #no leading *
> +                     WARN("Block comments use * on subsequent lines\n" . 
> $hereprev);
> +             }
> +
> +# Block comments use */ on trailing lines
> +             if ($rawline !~ address@hidden \t]*\*/[ \t]*$@ &&       
> #trailing */
> +                 $rawline !~ address@hidden/\*.*\*/[ \t]*$@ &&       #inline 
> /*...*/
> +                 $rawline !~ address@hidden,}/[ \t]*$@ &&    #trailing **/
> +                 $rawline =~ address@hidden \t]*.+\*\/[ \t]*$@) {    #non 
> blank */
> +                     WARN("Block comments use a trailing */ on a separate 
> line\n" . $herecurr);
> +             }
> +
> +# Block comment * alignment
> +             if ($prevline =~ /$;[ \t]*$/ &&                 #ends in comment
> +                 $line =~ /^\+[ \t]*$;/ &&                   #leading comment
> +                 $rawline =~ /^\+[ \t]*\*/ &&                #leading *
> +                 (($prevrawline =~ /^\+.*?\/\*/ &&           #leading /*
> +                   $prevrawline !~ /\*\/[ \t]*$/) ||         #no trailing */
> +                  $prevrawline =~ /^\+[ \t]*\*/)) {          #leading *
> +                     my $oldindent;
> +                     $prevrawline =~ address@hidden([ \t]*/?)\*@;
> +                     if (defined($1)) {
> +                             $oldindent = expand_tabs($1);
> +                     } else {
> +                             $prevrawline =~ address@hidden(.*/?)\*@;
> +                             $oldindent = expand_tabs($1);
> +                     }
> +                     $rawline =~ address@hidden([ \t]*)\*@;
> +                     my $newindent = $1;
> +                     $newindent = expand_tabs($newindent);
> +                     if (length($oldindent) ne length($newindent)) {
> +                             WARN("Block comments should align the * on each 
> line\n" . $hereprev);
> +                     }
> +             }
> +
>  # Check for potential 'bare' types
>               my ($stat, $cond, $line_nr_next, $remain_next, $off_next,
>                   $realline_next);

With a corrected commit message:
Reviewed-by: Markus Armbruster <address@hidden>