Re: [RFC/PATCH] Update coding standard to avoid ungrepable printkformat strings

[Stefan Richter]

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/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/