[v2] CODING_STYLE: Define our preferred form for multiline comments

Message ID 20180611141716.3813-1-peter.maydell@linaro.org
State Superseded
Headers show
Series
  • [v2] CODING_STYLE: Define our preferred form for multiline comments
Related show

Commit Message

Peter Maydell June 11, 2018, 2:17 p.m.
The codebase has a bit of a mix of different multiline
comment styles. State a preference for the Linux kernel
style:
    /*
     * Star on the left for each line.
     * Leading slash-star and trailing star-slash
     * each go on a line of their own.
     */

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
This is not my personal favourite, but seemed to be the
closest we had to consensus in the mail thread for v1;
I can live with it in order to avoid getting patches which
use the styles I like even less :-)
---
 CODING_STYLE | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

-- 
2.17.1

Comments

Eric Blake June 11, 2018, 2:21 p.m. | #1
On 06/11/2018 09:17 AM, Peter Maydell wrote:
> The codebase has a bit of a mix of different multiline

> comment styles. State a preference for the Linux kernel

> style:

>      /*

>       * Star on the left for each line.

>       * Leading slash-star and trailing star-slash

>       * each go on a line of their own.

>       */

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

> This is not my personal favourite, but seemed to be the

> closest we had to consensus in the mail thread for v1;

> I can live with it in order to avoid getting patches which

> use the styles I like even less :-)


Ditto that sentiment.

> ---

>   CODING_STYLE | 17 +++++++++++++++++

>   1 file changed, 17 insertions(+)

> 

> diff --git a/CODING_STYLE b/CODING_STYLE

> index 12ba58ee293..2d84f5f26d1 100644

> --- a/CODING_STYLE

> +++ b/CODING_STYLE

> @@ -124,6 +124,23 @@ We use traditional C-style /* */ comments and avoid // comments.

>   Rationale: The // form is valid in C99, so this is purely a matter of

>   consistency of style. The checkpatch script will warn you about this.

>   

> +Multiline comments blocks should have a row of stars on the left,


s/comments/comment/

> +and the initial /* and terminating */ both on their own lines:

> +    /*

> +     * like

> +     * this

> +     */

> +This is the same format required by the Linux kernel coding style.

> +

> +(Some of the existing comments in the codebase use the GNU Coding

> +Standards form which does not have stars on the left, or other

> +variations; avoid these when writing new comments, but don't worry

> +about converting to the preferred form unless you're editing that

> +comment anyway.)

> +

> +Rationale: Consistency, and ease of visually picking out a multiline

> +comment from the surrounding code.

> +


With the grammar fix,

Reviewed-by: Eric Blake <eblake@redhat.com>


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Cornelia Huck June 11, 2018, 2:57 p.m. | #2
On Mon, 11 Jun 2018 15:17:16 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> The codebase has a bit of a mix of different multiline

> comment styles. State a preference for the Linux kernel

> style:

>     /*

>      * Star on the left for each line.

>      * Leading slash-star and trailing star-slash

>      * each go on a line of their own.

>      */

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

> This is not my personal favourite, but seemed to be the

> closest we had to consensus in the mail thread for v1;

> I can live with it in order to avoid getting patches which

> use the styles I like even less :-)

> ---

>  CODING_STYLE | 17 +++++++++++++++++

>  1 file changed, 17 insertions(+)


Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Markus Armbruster June 11, 2018, 4:02 p.m. | #3
Peter Maydell <peter.maydell@linaro.org> writes:

> The codebase has a bit of a mix of different multiline

> comment styles. State a preference for the Linux kernel

> style:

>     /*

>      * Star on the left for each line.

>      * Leading slash-star and trailing star-slash

>      * each go on a line of their own.

>      */

>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Markus Armbruster <armbru@redhat.com>
Alex Williamson June 11, 2018, 4:15 p.m. | #4
On Mon, 11 Jun 2018 15:17:16 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> The codebase has a bit of a mix of different multiline

> comment styles. State a preference for the Linux kernel

> style:

>     /*

>      * Star on the left for each line.

>      * Leading slash-star and trailing star-slash

>      * each go on a line of their own.

>      */

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

> This is not my personal favourite, but seemed to be the

> closest we had to consensus in the mail thread for v1;

> I can live with it in order to avoid getting patches which

> use the styles I like even less :-)

> ---

>  CODING_STYLE | 17 +++++++++++++++++

>  1 file changed, 17 insertions(+)

> 

> diff --git a/CODING_STYLE b/CODING_STYLE

> index 12ba58ee293..2d84f5f26d1 100644

> --- a/CODING_STYLE

> +++ b/CODING_STYLE

> @@ -124,6 +124,23 @@ We use traditional C-style /* */ comments and avoid // comments.

>  Rationale: The // form is valid in C99, so this is purely a matter of

>  consistency of style. The checkpatch script will warn you about this.

>  

> +Multiline comments blocks should have a row of stars on the left,

> +and the initial /* and terminating */ both on their own lines:

> +    /*

> +     * like

> +     * this

> +     */

> +This is the same format required by the Linux kernel coding style.

> +

> +(Some of the existing comments in the codebase use the GNU Coding

> +Standards form which does not have stars on the left, or other

> +variations; avoid these when writing new comments, but don't worry

> +about converting to the preferred form unless you're editing that

> +comment anyway.)

> +

> +Rationale: Consistency, and ease of visually picking out a multiline

> +comment from the surrounding code.

> +

>  8. trace-events style

>  

>  8.1 0x prefix


Much preferred, thanks  (tabs next? ;)

Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Thomas Huth June 11, 2018, 4:24 p.m. | #5
On 11.06.2018 16:17, Peter Maydell wrote:
> The codebase has a bit of a mix of different multiline

> comment styles. State a preference for the Linux kernel

> style:

>     /*

>      * Star on the left for each line.

>      * Leading slash-star and trailing star-slash

>      * each go on a line of their own.

>      */

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

> This is not my personal favourite, but seemed to be the

> closest we had to consensus in the mail thread for v1;

> I can live with it in order to avoid getting patches which

> use the styles I like even less :-)

> ---

>  CODING_STYLE | 17 +++++++++++++++++

>  1 file changed, 17 insertions(+)

> 

> diff --git a/CODING_STYLE b/CODING_STYLE

> index 12ba58ee293..2d84f5f26d1 100644

> --- a/CODING_STYLE

> +++ b/CODING_STYLE

> @@ -124,6 +124,23 @@ We use traditional C-style /* */ comments and avoid // comments.

>  Rationale: The // form is valid in C99, so this is purely a matter of

>  consistency of style. The checkpatch script will warn you about this.

>  

> +Multiline comments blocks should have a row of stars on the left,

> +and the initial /* and terminating */ both on their own lines:

> +    /*

> +     * like

> +     * this

> +     */

> +This is the same format required by the Linux kernel coding style.

> +

> +(Some of the existing comments in the codebase use the GNU Coding

> +Standards form which does not have stars on the left, or other

> +variations; avoid these when writing new comments, but don't worry

> +about converting to the preferred form unless you're editing that

> +comment anyway.)

> +

> +Rationale: Consistency, and ease of visually picking out a multiline

> +comment from the surrounding code.


s/multiline/multi-line/g maybe? That seems to be the more common
spelling...? Anyway:

Reviewed-by: Thomas Huth <thuth@redhat.com>
John Snow June 11, 2018, 4:31 p.m. | #6
On 06/11/2018 10:17 AM, Peter Maydell wrote:
> The codebase has a bit of a mix of different multiline

> comment styles. State a preference for the Linux kernel

> style:

>     /*

>      * Star on the left for each line.

>      * Leading slash-star and trailing star-slash

>      * each go on a line of their own.

>      */

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

> This is not my personal favourite, but seemed to be the

> closest we had to consensus in the mail thread for v1;

> I can live with it in order to avoid getting patches which

> use the styles I like even less :-)


"Same."

Well, at least it's not asymmetric.

Reviewed-by: John Snow <jsnow@redhat.com>


> ---

>  CODING_STYLE | 17 +++++++++++++++++

>  1 file changed, 17 insertions(+)

> 

> diff --git a/CODING_STYLE b/CODING_STYLE

> index 12ba58ee293..2d84f5f26d1 100644

> --- a/CODING_STYLE

> +++ b/CODING_STYLE

> @@ -124,6 +124,23 @@ We use traditional C-style /* */ comments and avoid // comments.

>  Rationale: The // form is valid in C99, so this is purely a matter of

>  consistency of style. The checkpatch script will warn you about this.

>  

> +Multiline comments blocks should have a row of stars on the left,

> +and the initial /* and terminating */ both on their own lines:

> +    /*

> +     * like

> +     * this

> +     */

> +This is the same format required by the Linux kernel coding style.

> +

> +(Some of the existing comments in the codebase use the GNU Coding

> +Standards form which does not have stars on the left, or other

> +variations; avoid these when writing new comments, but don't worry

> +about converting to the preferred form unless you're editing that

> +comment anyway.)

> +

> +Rationale: Consistency, and ease of visually picking out a multiline

> +comment from the surrounding code.

> +

>  8. trace-events style

>  

>  8.1 0x prefix

>
Peter Maydell June 11, 2018, 4:40 p.m. | #7
On 11 June 2018 at 17:15, Alex Williamson <alex.williamson@redhat.com> wrote:
> Much preferred, thanks  (tabs next? ;)


CODING_STYLE already mandates spaces-only, no-tabs...

thanks
-- PMM
Alex Williamson June 11, 2018, 4:49 p.m. | #8
On Mon, 11 Jun 2018 17:40:35 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 11 June 2018 at 17:15, Alex Williamson <alex.williamson@redhat.com> wrote:

> > Much preferred, thanks  (tabs next? ;)  

> 

> CODING_STYLE already mandates spaces-only, no-tabs...


That was a joke, note the smiley.  I know, I just don't like it.
Thanks,

Alex
Richard Henderson June 12, 2018, 5:30 p.m. | #9
On 06/11/2018 04:17 AM, Peter Maydell wrote:
> The codebase has a bit of a mix of different multiline

> comment styles. State a preference for the Linux kernel

> style:

>     /*

>      * Star on the left for each line.

>      * Leading slash-star and trailing star-slash

>      * each go on a line of their own.

>      */

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

> This is not my personal favourite, but seemed to be the

> closest we had to consensus in the mail thread for v1;

> I can live with it in order to avoid getting patches which

> use the styles I like even less :-)


Honestly, I don't like this except for "important" comments.

A "small" comment, e.g. one that doesn't quite fit on a single line, now takes
4 lines instead of 2.  Which is really annoying and IMO tends to break flow.

If you don't like

  /* gnu
     style */

or

  /* whatever
   * this is */

could you live with

  // c99/c++
  // comments

(I know we currently deny those in checkpatch, but that's easy enough to remove
if we're changing policy.  I have no idea why we don't like them in the first
place.  Or other c99 features like mid-block or control declarations, for that
matter.)


r~
Philippe Mathieu-Daudé June 12, 2018, 5:41 p.m. | #10
On 06/12/2018 02:30 PM, Richard Henderson wrote:
> On 06/11/2018 04:17 AM, Peter Maydell wrote:

>> The codebase has a bit of a mix of different multiline

>> comment styles. State a preference for the Linux kernel

>> style:

>>     /*

>>      * Star on the left for each line.

>>      * Leading slash-star and trailing star-slash

>>      * each go on a line of their own.

>>      */

>>

>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>> ---

>> This is not my personal favourite, but seemed to be the

>> closest we had to consensus in the mail thread for v1;

>> I can live with it in order to avoid getting patches which

>> use the styles I like even less :-)

> 

> Honestly, I don't like this except for "important" comments.

> 

> A "small" comment, e.g. one that doesn't quite fit on a single line, now takes

> 4 lines instead of 2.  Which is really annoying and IMO tends to break flow.

> 

> If you don't like

> 

>   /* gnu

>      style */

> 

> or

> 

>   /* whatever

>    * this is */

> 

> could you live with

> 

>   // c99/c++

>   // comments

> 

> (I know we currently deny those in checkpatch, but that's easy enough to remove

> if we're changing policy.  I have no idea why we don't like them in the first

> place.  Or other c99 features like mid-block or control declarations, for that

> matter.)


+1 for control declarations :) Code is easier to read and keep focus on.

3.0 release is a good opportunity to establish such disruptive changes.
Thomas Huth June 12, 2018, 6:12 p.m. | #11
On 12.06.2018 19:30, Richard Henderson wrote:
> On 06/11/2018 04:17 AM, Peter Maydell wrote:

>> The codebase has a bit of a mix of different multiline

>> comment styles. State a preference for the Linux kernel

>> style:

>>     /*

>>      * Star on the left for each line.

>>      * Leading slash-star and trailing star-slash

>>      * each go on a line of their own.

>>      */

>>

>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>> ---

>> This is not my personal favourite, but seemed to be the

>> closest we had to consensus in the mail thread for v1;

>> I can live with it in order to avoid getting patches which

>> use the styles I like even less :-)

> 

> Honestly, I don't like this except for "important" comments.

> 

> A "small" comment, e.g. one that doesn't quite fit on a single line, now takes

> 4 lines instead of 2.  Which is really annoying and IMO tends to break flow.

> 

> If you don't like

> 

>   /* gnu

>      style */

> 

> or

> 

>   /* whatever

>    * this is */

> 

> could you live with

> 

>   // c99/c++

>   // comments


FWIW:

+1 for one or two of those compact styles for two- or three-line comments.

 Thomas
Alex Williamson June 12, 2018, 6:47 p.m. | #12
On Tue, 12 Jun 2018 20:12:02 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 12.06.2018 19:30, Richard Henderson wrote:

> > On 06/11/2018 04:17 AM, Peter Maydell wrote:  

> >> The codebase has a bit of a mix of different multiline

> >> comment styles. State a preference for the Linux kernel

> >> style:

> >>     /*

> >>      * Star on the left for each line.

> >>      * Leading slash-star and trailing star-slash

> >>      * each go on a line of their own.

> >>      */

> >>

> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> >> ---

> >> This is not my personal favourite, but seemed to be the

> >> closest we had to consensus in the mail thread for v1;

> >> I can live with it in order to avoid getting patches which

> >> use the styles I like even less :-)  

> > 

> > Honestly, I don't like this except for "important" comments.

> > 

> > A "small" comment, e.g. one that doesn't quite fit on a single line, now takes

> > 4 lines instead of 2.  Which is really annoying and IMO tends to break flow.

> > 

> > If you don't like

> > 

> >   /* gnu

> >      style */

> > 

> > or

> > 

> >   /* whatever

> >    * this is */

> > 

> > could you live with

> > 

> >   // c99/c++

> >   // comments  

> 

> FWIW:

> 

> +1 for one or two of those compact styles for two- or three-line comments.


or four-line or single paragraphs or comments that rhyme with
"orange"...  If the comment is too large for a single line, then take
some time to say it more concisely, or maybe it does deserve enough
thought to frame a nice paragraph for it.  We're well into personal
style here, so either this is important enough to make some people
unhappy or we should leave it to maintainer preference and consistency
within a file/area.  Thanks,

Alex
Cornelia Huck June 13, 2018, 7:05 a.m. | #13
On Tue, 12 Jun 2018 12:47:29 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Tue, 12 Jun 2018 20:12:02 +0200

> Thomas Huth <thuth@redhat.com> wrote:

> 

> > On 12.06.2018 19:30, Richard Henderson wrote:  

> > > On 06/11/2018 04:17 AM, Peter Maydell wrote:    

> > >> The codebase has a bit of a mix of different multiline

> > >> comment styles. State a preference for the Linux kernel

> > >> style:

> > >>     /*

> > >>      * Star on the left for each line.

> > >>      * Leading slash-star and trailing star-slash

> > >>      * each go on a line of their own.

> > >>      */

> > >>

> > >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> > >> ---

> > >> This is not my personal favourite, but seemed to be the

> > >> closest we had to consensus in the mail thread for v1;

> > >> I can live with it in order to avoid getting patches which

> > >> use the styles I like even less :-)    

> > > 

> > > Honestly, I don't like this except for "important" comments.

> > > 

> > > A "small" comment, e.g. one that doesn't quite fit on a single line, now takes

> > > 4 lines instead of 2.  Which is really annoying and IMO tends to break flow.

> > > 

> > > If you don't like

> > > 

> > >   /* gnu

> > >      style */

> > > 

> > > or

> > > 

> > >   /* whatever

> > >    * this is */

> > > 

> > > could you live with

> > > 

> > >   // c99/c++

> > >   // comments    

> > 

> > FWIW:

> > 

> > +1 for one or two of those compact styles for two- or three-line comments.  

> 

> or four-line or single paragraphs or comments that rhyme with

> "orange"...  If the comment is too large for a single line, then take

> some time to say it more concisely, or maybe it does deserve enough

> thought to frame a nice paragraph for it.  We're well into personal

> style here, so either this is important enough to make some people

> unhappy or we should leave it to maintainer preference and consistency

> within a file/area.  Thanks,


+1 to maintainer judgment and consistency.

(And FWIW, c++ style comments are what I use when I comment something
out for debugging, so I don't like them in regular code :)
Peter Maydell June 13, 2018, 12:30 p.m. | #14
On 12 June 2018 at 18:30, Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 06/11/2018 04:17 AM, Peter Maydell wrote:

>> The codebase has a bit of a mix of different multiline

>> comment styles. State a preference for the Linux kernel

>> style:

>>     /*

>>      * Star on the left for each line.

>>      * Leading slash-star and trailing star-slash

>>      * each go on a line of their own.

>>      */

>>

>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>> ---

>> This is not my personal favourite, but seemed to be the

>> closest we had to consensus in the mail thread for v1;

>> I can live with it in order to avoid getting patches which

>> use the styles I like even less :-)

>

> Honestly, I don't like this except for "important" comments.

>

> A "small" comment, e.g. one that doesn't quite fit on a single line, now takes

> 4 lines instead of 2.  Which is really annoying and IMO tends to break flow.


That's why I prefer
 /* two
  * lines
  */
style over kernel-style. But apparently nobody else does...

thanks
-- PMM
Stefan Hajnoczi June 13, 2018, 1:31 p.m. | #15
On Mon, Jun 11, 2018 at 03:17:16PM +0100, Peter Maydell wrote:
> The codebase has a bit of a mix of different multiline

> comment styles. State a preference for the Linux kernel

> style:

>     /*

>      * Star on the left for each line.

>      * Leading slash-star and trailing star-slash

>      * each go on a line of their own.

>      */

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

> This is not my personal favourite, but seemed to be the

> closest we had to consensus in the mail thread for v1;

> I can live with it in order to avoid getting patches which

> use the styles I like even less :-)

> ---

>  CODING_STYLE | 17 +++++++++++++++++

>  1 file changed, 17 insertions(+)


Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
John Snow June 13, 2018, 4:55 p.m. | #16
On 06/13/2018 08:30 AM, Peter Maydell wrote:
> On 12 June 2018 at 18:30, Richard Henderson

> <richard.henderson@linaro.org> wrote:

>> On 06/11/2018 04:17 AM, Peter Maydell wrote:

>>> The codebase has a bit of a mix of different multiline

>>> comment styles. State a preference for the Linux kernel

>>> style:

>>>     /*

>>>      * Star on the left for each line.

>>>      * Leading slash-star and trailing star-slash

>>>      * each go on a line of their own.

>>>      */

>>>

>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>>> ---

>>> This is not my personal favourite, but seemed to be the

>>> closest we had to consensus in the mail thread for v1;

>>> I can live with it in order to avoid getting patches which

>>> use the styles I like even less :-)

>>

>> Honestly, I don't like this except for "important" comments.

>>

>> A "small" comment, e.g. one that doesn't quite fit on a single line, now takes

>> 4 lines instead of 2.  Which is really annoying and IMO tends to break flow.

> 

> That's why I prefer

>  /* two

>   * lines

>   */

> style over kernel-style. But apparently nobody else does...


The same reasoning could be used to justify

/* two
 * lines */

as it's ... actually just two lines. I think people don't seem to like
this much either (why? does it look 'naked' on the end?) and seemingly
the asymmetry is a bit of a killer for the isolated close-comment style:

/* asymmetric
 * three lines
 */

so we seemed to have arrived on the vertically symmetric:

/*
 * four
 * lines
 */

style. I think this is a pretty reasonable compromise with the tastes of
the various maintainers that have chimed in so far -- it seems to be
nobody's favorite but everyone's second choice :)

If there is concern this will balloon "almost" single line comments into
four lines, I think the overall strategy of being either more concise
(or being 70 characters more detailed!) is probably reasonable enough.

...Overall I think it probably doesn't matter too much as long as
comments in each individual file are consistent.

It would only begin to matter terribly much if we actually decided we
wanted to do a doxygen-style doc generation for our internal APIs for
compatibility with, say, fancier IDEs than vim/emacs.

As it stands, we're pretty inconsistent about which exact style we apply
when we "document" internal functions -- sometimes we document the
header, sometimes the implementation, sometimes both (but differently!)
and always with different styles all over the place. That's the real
problem, IMO.

For non-javadoc style comments that aren't intended to document a
function in some methodical way, I think as long as it looks nice in the
file as deemed by the maintainer and it's consistent, it's probably fine.

> 

> thanks

> -- PMM

>

Now I can say after working at Red Hat for four years that I've finally
had a real life "tabs vs spaces" argument, too. I guess I had a good run
without :)

--js
Peter Maydell June 14, 2018, 10:46 a.m. | #17
On 13 June 2018 at 17:55, John Snow <jsnow@redhat.com> wrote:
> The same reasoning could be used to justify

>

> /* two

>  * lines */

>

> as it's ... actually just two lines. I think people don't seem to like

> this much either (why? does it look 'naked' on the end?)


I dislike the way it breaks up the line of stars. For me it is the
/*
 *
 */
shape that defines a multiline comment, and where exactly the text is
on the RHS of it is not important to my sense of visual neatness :-)

> It would only begin to matter terribly much if we actually decided we

> wanted to do a doxygen-style doc generation for our internal APIs for

> compatibility with, say, fancier IDEs than vim/emacs.


We ought to do that at some point -- I had some prototype patches
for it. Doc-comment comments always start /** on a line of its own,
though.

> As it stands, we're pretty inconsistent about which exact style we apply

> when we "document" internal functions -- sometimes we document the

> header, sometimes the implementation, sometimes both (but differently!)

> and always with different styles all over the place. That's the real

> problem, IMO.


IMHO -- global functions should always be documented in the header
with the prototype, and any new global function should get a
doc comment (I require this for code I review...) I should be able
to read about the API your code exposes to the rest of QEMU purely
by looking at your headers.

thanks
-- PMM
John Snow June 14, 2018, 8:11 p.m. | #18
On 06/14/2018 06:46 AM, Peter Maydell wrote:
> On 13 June 2018 at 17:55, John Snow <jsnow@redhat.com> wrote:

>> The same reasoning could be used to justify

>>

>> /* two

>>  * lines */

>>

>> as it's ... actually just two lines. I think people don't seem to like

>> this much either (why? does it look 'naked' on the end?)

> 

> I dislike the way it breaks up the line of stars. For me it is the

> /*

>  *

>  */

> shape that defines a multiline comment, and where exactly the text is

> on the RHS of it is not important to my sense of visual neatness :-)

> 


Yours does look an awful lot more symmetrical once you remove the text,
yeah.

*cough* I hate the way it looks too, but C99 comments have a few things
going for them:

// A multi-line comment block like this has no extra lines and every
// line in the comment is prefaced individually which aids grep
// readability, while maintained good vertical symmetry.

I think we hate C99 comments, though? Certainly we don't use them at all
right now, so it's not a good fit.

>> It would only begin to matter terribly much if we actually decided we

>> wanted to do a doxygen-style doc generation for our internal APIs for

>> compatibility with, say, fancier IDEs than vim/emacs.

> 

> We ought to do that at some point -- I had some prototype patches

> for it. Doc-comment comments always start /** on a line of its own,

> though.

> 


I'd love this! I love vim/emacs, but my usage of it is not wizard-tier
and in the past when working on large C++ projects I have benefited from
the magical refactoring click-buttons, tool-tips and etc. These
operations are infrequent enough that I believe it's reasonable to not
know how to do them in traditional CLI editors. If we want to lure in
new contributors, maybe this could sweeten the pot a bit?

Rigorous, mechanically verifiable function documentation is quite nice
to have in these cases. It'd be nice in general, really. It would go a
long way to help us attract less "hardcore" developers implementing
devices and features for QEMU without such a steep onboarding curve.

Do you have a proposed standard / do we have some consensus on which
generator tool or doc format we'd most like to see in QEMU? I could put
in some elbow grease to shine up the block layer if so...

>> As it stands, we're pretty inconsistent about which exact style we apply

>> when we "document" internal functions -- sometimes we document the

>> header, sometimes the implementation, sometimes both (but differently!)

>> and always with different styles all over the place. That's the real

>> problem, IMO.

> 

> IMHO -- global functions should always be documented in the header

> with the prototype, and any new global function should get a

> doc comment (I require this for code I review...) I should be able

> to read about the API your code exposes to the rest of QEMU purely

> by looking at your headers.

> 


This makes sense, though the way C code is laid out makes it unfortunate
you don't get to see the same comment right beside the implementation if
that's what you're working on -- but I suppose this is why we have tabs,
multi-monitors and IDEs with tooltips.

> thanks

> -- PMM

>
Philippe Mathieu-Daudé June 15, 2018, 2:52 a.m. | #19
On 06/14/2018 05:11 PM, John Snow wrote:
> On 06/14/2018 06:46 AM, Peter Maydell wrote:

>> On 13 June 2018 at 17:55, John Snow <jsnow@redhat.com> wrote:

>>> The same reasoning could be used to justify

>>>

>>> /* two

>>>  * lines */

>>>

>>> as it's ... actually just two lines. I think people don't seem to like

>>> this much either (why? does it look 'naked' on the end?)

>>

>> I dislike the way it breaks up the line of stars. For me it is the

>> /*

>>  *

>>  */

>> shape that defines a multiline comment, and where exactly the text is

>> on the RHS of it is not important to my sense of visual neatness :-)

>>

> 

> Yours does look an awful lot more symmetrical once you remove the text,

> yeah.

> 

> *cough* I hate the way it looks too, but C99 comments have a few things

> going for them:

> 

> // A multi-line comment block like this has no extra lines and every

> // line in the comment is prefaced individually which aids grep

> // readability, while maintained good vertical symmetry.

> 

> I think we hate C99 comments, though? Certainly we don't use them at all

> right now, so it's not a good fit.

> 

>>> It would only begin to matter terribly much if we actually decided we

>>> wanted to do a doxygen-style doc generation for our internal APIs for

>>> compatibility with, say, fancier IDEs than vim/emacs.

>>

>> We ought to do that at some point -- I had some prototype patches

>> for it. Doc-comment comments always start /** on a line of its own,

>> though.

>>

> 

> I'd love this! I love vim/emacs, but my usage of it is not wizard-tier

> and in the past when working on large C++ projects I have benefited from

> the magical refactoring click-buttons, tool-tips and etc. These

> operations are infrequent enough that I believe it's reasonable to not

> know how to do them in traditional CLI editors. If we want to lure in

> new contributors, maybe this could sweeten the pot a bit?

> 

> Rigorous, mechanically verifiable function documentation is quite nice

> to have in these cases. It'd be nice in general, really. It would go a

> long way to help us attract less "hardcore" developers implementing

> devices and features for QEMU without such a steep onboarding curve.

> 

> Do you have a proposed standard / do we have some consensus on which

> generator tool or doc format we'd most like to see in QEMU? I could put

> in some elbow grease to shine up the block layer if so...

> 

>>> As it stands, we're pretty inconsistent about which exact style we apply

>>> when we "document" internal functions -- sometimes we document the

>>> header, sometimes the implementation, sometimes both (but differently!)

>>> and always with different styles all over the place. That's the real

>>> problem, IMO.

>>

>> IMHO -- global functions should always be documented in the header

>> with the prototype, and any new global function should get a

>> doc comment (I require this for code I review...) I should be able

>> to read about the API your code exposes to the rest of QEMU purely

>> by looking at your headers.

>>

> 

> This makes sense, though the way C code is laid out makes it unfortunate

> you don't get to see the same comment right beside the implementation if

> that's what you're working on -- but I suppose this is why we have tabs,

> multi-monitors and IDEs with tooltips.


Thanks to tabs we don't need multi-monitors of 1600+ resolution to fit
80 chars per line.
Thomas Huth June 15, 2018, 4:43 a.m. | #20
On 14.06.2018 22:11, John Snow wrote:
> 

> On 06/14/2018 06:46 AM, Peter Maydell wrote:

[...]
> 

> *cough* I hate the way it looks too, but C99 comments have a few things

> going for them:

> 

> // A multi-line comment block like this has no extra lines and every

> // line in the comment is prefaced individually which aids grep

> // readability, while maintained good vertical symmetry.

> 

> I think we hate C99 comments, though? Certainly we don't use them at all

> right now, so it's not a good fit.


But why do we hate them? I remember there were some reasons for not
using them 10 or 15 years ago (some old C-compilers still did not
support them yet), but today? ... it's likely just a legacy feeling.
Maybe it's time to get over that feeling now?

>>> It would only begin to matter terribly much if we actually decided we

>>> wanted to do a doxygen-style doc generation for our internal APIs for

>>> compatibility with, say, fancier IDEs than vim/emacs.

[...]
> Do you have a proposed standard / do we have some consensus on which

> generator tool or doc format we'd most like to see in QEMU? I could put

> in some elbow grease to shine up the block layer if so...


I remember that some years ago, somebody (I forgot who it was, sorry)
once told me that we should use the same format in QEMU as in glib, i.e.
GTK-Doc. But citing the GTK-Doc homepage: "For a more polished
general-purpose documentation tool you may want to look at Doxygen" - so
maybe that's the better choice instead.

 Thomas
Peter Maydell June 15, 2018, 8:48 a.m. | #21
On 15 June 2018 at 05:43, Thomas Huth <thuth@redhat.com> wrote:
> On 14.06.2018 22:11, John Snow wrote:

>> Do you have a proposed standard / do we have some consensus on which

>> generator tool or doc format we'd most like to see in QEMU? I could put

>> in some elbow grease to shine up the block layer if so...

>

> I remember that some years ago, somebody (I forgot who it was, sorry)

> once told me that we should use the same format in QEMU as in glib, i.e.

> GTK-Doc. But citing the GTK-Doc homepage: "For a more polished

> general-purpose documentation tool you may want to look at Doxygen" - so

> maybe that's the better choice instead.


The last round of email threads on this basically concluded that
we should use sphinx, which is what the kernel uses. So kernel-doc
comment format, and rst for standalone docs files. Some tidying
up of our current doc comments would be required (which are a bit
of a mishmash of styles plus syntax errors because we've never
done any kind of processing of them to keep us from making mistakes).

thanks
-- PMM
Markus Armbruster June 15, 2018, 12:45 p.m. | #22
Peter Maydell <peter.maydell@linaro.org> writes:

> On 13 June 2018 at 17:55, John Snow <jsnow@redhat.com> wrote:

[...]
>> It would only begin to matter terribly much if we actually decided we

>> wanted to do a doxygen-style doc generation for our internal APIs for

>> compatibility with, say, fancier IDEs than vim/emacs.

>

> We ought to do that at some point -- I had some prototype patches

> for it. Doc-comment comments always start /** on a line of its own,

> though.

>

>> As it stands, we're pretty inconsistent about which exact style we apply

>> when we "document" internal functions -- sometimes we document the

>> header, sometimes the implementation, sometimes both (but differently!)

>> and always with different styles all over the place. That's the real

>> problem, IMO.

>

> IMHO -- global functions should always be documented in the header

> with the prototype, and any new global function should get a

> doc comment (I require this for code I review...) I should be able

> to read about the API your code exposes to the rest of QEMU purely

> by looking at your headers.


Putting the function contract far from the actual function is a proven
way to end up with a contract that is far from what the function
actually does.

With a documentation generator such as Sphinx, the usual argument for
putting the contracts in headers "I want API documentation in one place,
without clutter" carries a lot less weight: the generated documentation
is exactly that.

Patch

diff --git a/CODING_STYLE b/CODING_STYLE
index 12ba58ee293..2d84f5f26d1 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -124,6 +124,23 @@  We use traditional C-style /* */ comments and avoid // comments.
 Rationale: The // form is valid in C99, so this is purely a matter of
 consistency of style. The checkpatch script will warn you about this.
 
+Multiline comments blocks should have a row of stars on the left,
+and the initial /* and terminating */ both on their own lines:
+    /*
+     * like
+     * this
+     */
+This is the same format required by the Linux kernel coding style.
+
+(Some of the existing comments in the codebase use the GNU Coding
+Standards form which does not have stars on the left, or other
+variations; avoid these when writing new comments, but don't worry
+about converting to the preferred form unless you're editing that
+comment anyway.)
+
+Rationale: Consistency, and ease of visually picking out a multiline
+comment from the surrounding code.
+
 8. trace-events style
 
 8.1 0x prefix