public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org, Peter Zijlstra <peterz@infradead.org>,
	Paul McKenney <paulmck@linux.vnet.ibm.com>,
	John Stultz <john.stultz@linaro.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Andy Lutomirski <luto@kernel.org>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Will Deacon <will.deacon@arm.com>,
	Mark Brown <broonie@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>
Subject: Re: [patch 2/2] Documentation/process: Add tip tree handbook
Date: Thu, 8 Nov 2018 08:05:47 +0100	[thread overview]
Message-ID: <20181108070547.GA20032@gmail.com> (raw)
In-Reply-To: <20181107171149.165693799@linutronix.de>


* Thomas Gleixner <tglx@linutronix.de> wrote:

> +Coding style notes
> +------------------
> +
> +Comment style
> +^^^^^^^^^^^^^
> +
> +Sentences in comments start with a uppercase letter.
> +
> +Single line comments::
> +
> +	/* This is a single line comment */
> +
> +Multi-line comments::
> +
> +	/*
> +	 * This is a properly formatted
> +	 * multi-line comment.
> +	 *
> +	 * Larger multi-line comments should be split into paragraphs.
> +	 */
> +
> +No tail comments:
> +
> +  Please refrain from using tail comments. Tail comments disturb the
> +  reading flow in almost all contexts, but especially in code::
> +
> +  	if (somecondition_is_true) /* Don't put a comment here */
> +		dostuff(); /* Neither here */
> +
> +	seed = MAGIC_CONSTANT; /* Nor here */
> +
> +  Use freestanding comments instead::
> +
> +	/* This condition is not obvious without a comment */
> +	if (somecondition_is_true) {
> +		/* This really needs to be documented */
> +		dostuff();
> +	}
> +
> +	/* This magic initialization needs a comment. Maybe not? */
> +	seed = MAGIC_CONSTANT;

Yeah, so I think a better way to visualize and explain the 'no tail 
comments' guideline in -tip is not these more complex code flows, but the 
totally simple linear flows of statements.

With tail comments the code looks like this:

	res = dostuff(); /* We explain something here. */

	seed = 1; /* Another explanation. */

	mod_timer(&our_object->our_timer, jiffies + OUR_INTERVAL); /* We like to talk */

	res = check_stuff(our_object); /* We explain something here. */
	if (res)
		return -EINVAL;

	interval = nontrivial_calculation(); /* Another explanation. */

	mod_timer(&our_object->our_timer, jiffies + interval); /* This doesn't race, because. */

... while with freestanding comments it's:

	/* We explain something here: */
	res = check_stuff(our_object);
	if (res)
		return -EINVAL;

	/* Another explanation: */
	interval = nontrivial_calculation();

	/* This doesn't race with init_our_stuff(), because: */
	mod_timer(&our_object->our_timer, jiffies + interval);

This comment placement style has several advantages:

  - Comments precede actual code - while in tail comments it's exactly
    the wrong way around.

  - We don't create over-long lines nor artificially short tail comments 
    just because we were trying to stay within the col80 limit with the 
    "this doesn't race" comment. The full-line comment was able to 
    explain that the race is with init_our_stuff().

  - Freestanding oneliner comments are much better aligned as well: note 
    how ever comment starts at the exact same column, making it very easy 
    to read (or not to read) these comments.

  - In short: predictable visual parsing rules and proper semantic 
    ordering of information is good, random joining of typographical 
    elements just to create marginally more compact source code is bad.

  - Just *look* at the tail comments example: it's a visually diffuse, 
    jumble of statements and misaligned comments with good structure.

Do you want me to send a delta patch, or an edited document?

Thanks,

	Ingo

  parent reply	other threads:[~2018-11-08  7:05 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-07 17:10 [patch 0/2] Documentation/process: Add subsystem/tree handbook Thomas Gleixner
2018-11-07 17:10 ` [patch 1/2] Documentation/process: Add maintainer handbooks section Thomas Gleixner
2018-11-07 17:10 ` [patch 2/2] Documentation/process: Add tip tree handbook Thomas Gleixner
2018-11-07 17:44   ` Thomas Gleixner
2018-11-07 19:57     ` Paul E. McKenney
2018-11-07 19:38   ` Paul E. McKenney
2018-11-08  7:05   ` Ingo Molnar [this message]
2018-11-08  7:14     ` Ingo Molnar
2018-11-08  7:19   ` Ingo Molnar
2018-11-08  7:30   ` Ingo Molnar
2018-11-08  7:40   ` Ingo Molnar
2018-11-08  9:12     ` Peter Zijlstra
2018-11-08 11:05       ` Greg Kroah-Hartman
2018-11-08 17:19       ` Dan Williams
2018-11-08 17:24         ` Borislav Petkov
2018-11-08 17:40         ` Paul E. McKenney
2018-11-08 19:58           ` Thomas Gleixner
2018-11-08 20:05             ` Paul E. McKenney
2018-11-08 21:06             ` Greg KH
2018-11-08 21:08               ` Greg KH
2018-11-08 22:38               ` Thomas Gleixner
2018-11-08 20:14         ` Theodore Y. Ts'o
2018-11-08 20:22           ` Thomas Gleixner
2018-11-08 21:04           ` Greg KH
2018-11-08 22:19             ` Theodore Y. Ts'o
2018-11-08 22:33               ` Greg KH
2018-11-08 22:56           ` Dan Williams
2018-11-08  7:46   ` Ingo Molnar
2018-11-08  8:04   ` Ingo Molnar
2018-11-08  8:13   ` Ingo Molnar
2018-11-08  8:18   ` Ingo Molnar
2018-11-08  8:30   ` Ingo Molnar
2018-11-07 19:48 ` [patch 0/2] Documentation/process: Add subsystem/tree handbook Jonathan Corbet
2018-11-07 19:58   ` Dan Williams
2018-11-07 20:51     ` Thomas Gleixner
2018-11-08 14:49       ` Jonathan Corbet
2018-11-08 15:05         ` Peter Zijlstra
2018-11-08 15:19           ` Jonathan Corbet
2018-11-08 16:05             ` Paul E. McKenney
2018-11-08 16:21           ` Theodore Y. Ts'o
2018-11-08 16:32             ` Mark Brown
2018-11-08 17:32               ` Dan Williams
2018-11-13 23:15                 ` Mark Brown
2018-11-08 16:33             ` Jonathan Corbet
2018-11-08 19:46               ` Theodore Y. Ts'o
2018-11-08 15:49         ` Thomas Gleixner
2020-12-17 15:05           ` Borislav Petkov
2020-12-17 17:53             ` Jonathan Corbet
2020-12-17 17:58               ` Borislav Petkov
2018-11-12  5:52         ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2021-09-13 15:39 [PATCH 0/2] doc: Add tip maintainer's handbook Borislav Petkov
2021-09-13 15:39 ` [PATCH 2/2] Documentation/process: Add tip tree handbook Borislav Petkov

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=20181108070547.GA20032@gmail.com \
    --to=mingo@kernel.org \
    --cc=acme@redhat.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=broonie@kernel.org \
    --cc=corbet@lwn.net \
    --cc=dan.j.williams@intel.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=frederic@kernel.org \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=will.deacon@arm.com \
    --cc=x86@kernel.org \
    /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