qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Developers <qemu-devel@nongnu.org>,
	"patches@linaro.org" <patches@linaro.org>,
	Thomas Huth <thuth@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax
Date: Fri, 10 Aug 2018 10:34:14 +0200	[thread overview]
Message-ID: <9da04e7f-bd0c-9d50-fb7c-026b5aaa674c@redhat.com> (raw)
In-Reply-To: <CAFEAcA_rRW7qbtiM4c-sBVmQhFMSuKnVqU6+oXsdXPwJ3GKv6A@mail.gmail.com>

On 09/08/2018 19:03, Peter Maydell wrote:
> On 9 August 2018 at 17:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> I'm still not used to the leeading-/*-on-it's-own style,
>>> so having checkpatch catch my lapses is handy...
>>
>> ... if it's not what we are using, why enforce it?
> 
> See the enormous long threads on the recent changes to CODING_STYLE:
> https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg00696.html
> https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg02717.html
> 
> Basically, I wanted to rule out things like
> 
>   /* this
>      weirdness */
> 
> and lots of other people wanted (a) to not have
> 
>   /* this thing
>    * which I think is fine
>    */
> 
> and (b) to consistently define only one format as OK.
> 
> So I accepted having my personal preferred format not being
> permitted in order to get consensus on getting rid of the
> formats I think are really ugly :-)

This is one of the cases where we are decently consistent:

Lone "/*" or "/**": 9986 cases
	of which in the first column: 7617
	of which the first line in the file (license headers): 2834
	regex: ^[ \t]*/\*\*?[ \t]*$

"/*" with the first line of the comment: 11246
	of which in the first column: 4985
	of which the first line in the file: 97
	regex: ^[ \t]*/\*\*?+(?:(?!\*/).)+?$

License headers almost always have the "lone /*" format.  Apart from
license headers, 63% of the comments have the now-deprecated format.

Inside functions, 73% of the comments have the now-deprecated format.
Outside functions it's 50-50.  That's because there are 2024 doc
comments, which in turn are 50% of the comments that are 1) outside the
functions 2) using a lone "/*".

So my proposal, which is actually consistent with what QEMU is doing, is
the following:

1) the first line of a file should always be "/*", otherwise warn

2) a comment that starts with "/**" should have it on a lone line

3) every other multiline comment should start with
"/*<whitespace><something>"

Yes, there is overlap between QEMU and Linux developers, but really only
in a few subsystems (s390, pSeries, networking---which uses the "other"
comment style), and I don't see why we should pretend that QEMU and
Linux use similar coding styles.  In fact they couldn't be more
different: spaces vs. tabs, indent-4 vs. indent-8, camelcase struct
names with typedefs...  Basically the only thing that is the same is
lowercase for variable names and braces on the same line as the
statement.  Linux's checkpatch was a useful base not because Linux and
QEMU are similar, but only because of the complex expression parsing
stuff that really is the same for _any_ sane coding style (even GNU ;)).

Paolo

  reply	other threads:[~2018-08-10  8:34 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-09 16:00 [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax Peter Maydell
2018-08-09 16:43 ` Paolo Bonzini
2018-08-09 17:03   ` Peter Maydell
2018-08-10  8:34     ` Paolo Bonzini [this message]
2018-08-10  9:10       ` Peter Maydell
2018-08-10 12:45         ` Paolo Bonzini
2018-08-10 12:53           ` Peter Maydell
2018-08-10 17:06             ` Paolo Bonzini
2018-08-13  6:18           ` Thomas Huth
2018-08-13  8:01             ` Paolo Bonzini
2018-08-10  9:41       ` Markus Armbruster
2018-08-10 12:44         ` Paolo Bonzini
2018-08-10 13:19           ` Markus Armbruster
2018-08-10  6:22 ` Markus Armbruster
2018-08-10  9:07   ` Peter Maydell
2018-08-10 12:08     ` Markus Armbruster
2018-08-13  6:26 ` Thomas Huth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9da04e7f-bd0c-9d50-fb7c-026b5aaa674c@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=patches@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).