On 10 August 2018 at 07:22, Markus Armbruster <arm...@redhat.com> wrote: > Peter Maydell <peter.mayd...@linaro.org> 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 <peter.mayd...@linaro.org> >> --- >> 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 !~ m@^\+.*/\*.*\*/[ \t]*$@ && #inline /*...*/ >> + $rawline =~ m@^\+.*/\*\*?[ \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 =~ m@^(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 <arm...@redhat.com> thanks -- PMM