From: Stefan Richter <stefanr@s5r6.in-berlin.de>
To: Andi Kleen <andi@firstfloor.org>
Cc: linux-kernel@vger.kernel.org, torvalds@osdl.org
Subject: Re: [RFC/PATCH] Update coding standard to avoid ungrepable printk format strings
Date: Fri, 22 Feb 2008 14:53:07 +0100 [thread overview]
Message-ID: <47BED3C3.5020500@s5r6.in-berlin.de> (raw)
In-Reply-To: <20080222132612.GA11717@basil.nowhere.org>
Andi Kleen wrote:
> --- linux.orig/Documentation/CodingStyle
> +++ linux/Documentation/CodingStyle
> @@ -83,20 +83,32 @@ preferred limit.
> Statements longer than 80 columns will be broken into sensible chunks.
> Descendants are always substantially shorter than the parent and are placed
> substantially to the right. The same applies to function headers with a long
> -argument list. Long strings are as well broken into shorter strings. The
> -only exception to this is where exceeding 80 columns significantly increases
> -readability and does not hide information.
> +argument list.
>
> -void fun(int a, int b, int c)
> -{
> - if (condition)
> - printk(KERN_WARNING "Warning this is a long printk with "
> - "3 parameters a: %u b: %u "
> - "c: %u \n", a, b, c);
> - else
> - next_statement;
> +It is not recommended to break printk format strings into smaller strings.
Instead of a new recommendation (from now on we recommend something
contrary to what we required up until yesterday --- let's go unwrap
strings everywhere in the kernel now), how about simply saying that
printk format strings are not subject to the 80 column rule?
Or keep the old text and insert after "increases readability": "or
helps full-text searching". And delete the example code.
> +The problem with doing this is that it makes it much harder to grep
> +for the error messages in the source if they are split up over multiple
> +lines. And grepping for error messages is fairly important for debugging.
> +So for the special case of printk format strings (or formatting any other
> +user visible error message) the normal 80 character column rule
> +does not apply. Or alternatively it is ok to violate the indentation
> +rule for the format string only if that makes the end not exceed
> +80 characters. For example
> +
> +void function(void)
> +{
> + if (...) {
> + if (...) {
> + printk(
> + "very very long formatting string with argument %d and argument %d\n",
> + a, b);
> + }
> + }
> }
Here is one vote against this indentation exception.
PS: Could someone implement this for checkpatch.pl:
WARN_ON(hunk_is_in("Documentation/CodingStyle") &&
lines_added > lines_removed);
--
Stefan Richter
-=====-==--- --=- =-==-
http://arcgraph.de/sr/
next prev parent reply other threads:[~2008-02-22 13:53 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-02-22 13:26 Andi Kleen
2008-02-22 13:53 ` Stefan Richter [this message]
2008-02-22 14:01 ` Andi Kleen
2008-02-22 15:02 ` Alan Cox
2008-02-22 16:58 ` Joe Perches
2008-02-23 12:55 ` Christer Weinigel
2008-02-25 5:29 ` Andy Whitcroft
2008-02-25 5:30 ` Andy Whitcroft
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=47BED3C3.5020500@s5r6.in-berlin.de \
--to=stefanr@s5r6.in-berlin.de \
--cc=andi@firstfloor.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@osdl.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
all inboxes | Powered by JetHome®