[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline commen
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax |
Date: |
Fri, 10 Aug 2018 14:08:08 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Peter Maydell <address@hidden> writes:
> 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.
It confused me. Looks like I'm too easily confused today.
>> Reviewed-by: Markus Armbruster <address@hidden>
Feel free to use this R-by even without commit message improvements.
- Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax, (continued)
- Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax, Thomas Huth, 2018/08/13
- Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax, Paolo Bonzini, 2018/08/13
- Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax, Markus Armbruster, 2018/08/10
- Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax, Paolo Bonzini, 2018/08/10
- Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax, Markus Armbruster, 2018/08/10
Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax, Markus Armbruster, 2018/08/10
Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax, Thomas Huth, 2018/08/13