Messages in this thread | | | Date | Thu, 8 Nov 2018 09:30:51 +0100 | From | Ingo Molnar <> | Subject | Re: [patch 2/2] Documentation/process: Add tip tree handbook |
| |
* 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
| |