Peter Maydell <peter.mayd...@linaro.org> writes: > 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.
It confused me. Looks like I'm too easily confused today. >> Reviewed-by: Markus Armbruster <arm...@redhat.com> Feel free to use this R-by even without commit message improvements.