Re: [patch 2/2] Documentation/process: Add tip tree handbook

Ingo Molnar
2018-11-08T08:30:58Z

* 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