All of lore.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 09:30:51 +0100	[thread overview]
Message-ID: <20181108083051.GI20032@gmail.com> (raw)
In-Reply-To: <20181107171149.165693799@linutronix.de>


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

> +Line breaks
> +^^^^^^^^^^^
> +
> +Restricting line length to 80 characters makes deeply indented code hard to
> +read.  Consider breaking out code into helper functions to avoid excessive
> +line breaking.
> +
> +The 80 character rule is not a strict rule, so please use common sense when
> +breaking lines. Especially format strings should never be broken up.

Might make sense to explain that:

  + The reason for that rule is that if for example we have this printk line:
  +
  +		if (uh_oh()) {
  +			pr_info("Something really bad happened, danger"
  +				"danger, blue smoke reported!\n");
  +		}
  +
  + People would see this message in the syslog:
  +
  +   Thu Nov  8 09:22:33: Something really bad happened, dangerdanger, blue smoke reported!
  +
  + And chances are that in sheer panic they'd type the most distinctive 
  + part of that text as a search pattern for the kernel source tree:
  +
  +   $ git grep -i 'dangerdanger'
  +   $
  +
  + ... and they'd get absolutely no match on that string due to the 
  + col80 broken format string, and confusion and frustration would rise, 
  + in addition to growing amounts of blue smoke.
  +
  + We don't want that, so just write out the single line:

  +		if (uh_oh())
  +			pr_info("Something really bad happened, danger danger, blue smoke reported!\n");
  +
  + Also note two other advantages of writing it like this:
  +
  +  - We saved two curly braces.
  +  - We also added a proper space to 'danger danger' which was the original intended message.

?

> +
> +When splitting function declarations or function calls, then please align
> +the first argument in the second line with the first argument in the first
> +line::
> +
> +  static int long_function_name(struct foobar *barfoo, unsigned int id,
> +  				unsigned int offset)
> +  {
> +
> +  	if (!id) {
> +		ret = longer_function_name(barfoo, DEFAULT_BARFOO_ID,
> +					   offset);

BTW., in this particular case I think small violations of col80 rule are 
even more readable, i.e.:

> +		ret = longer_function_name(barfoo, DEFAULT_BARFOO_ID, offset);

And note that in this example we used 78 colums so we didn't even violate 
the col80 rule. ;-)

Thanks,

	Ingo

  parent reply	other threads:[~2018-11-08  8:30 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
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 [this message]
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=20181108083051.GI20032@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.