linux-kernel - Re: [RFC/PATCH] Update coding standard to avoid ungrepable printk format strings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <47BED3C3.5020500@s5r6.in-berlin.de>
Date:	Fri, 22 Feb 2008 14:53:07 +0100
From:	Stefan Richter <stefanr@...6.in-berlin.de>
To:	Andi Kleen <andi@...stfloor.org>
CC:	linux-kernel@...r.kernel.org, torvalds@...l.org
Subject: Re: [RFC/PATCH] Update coding standard to avoid ungrepable printk
 format strings

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/