[PATCH] checkpatch: Warn if trace_printk and friends are called

[PATCH] checkpatch: Warn if trace_printk and friends are called

[Nicolas Boichat]

trace_printk is meant as a debugging tool, and should not be
compiled into production code without specific debug Kconfig
options enabled, or source code changes, as indicated by the
warning that shows up on boot if any trace_printk is called:
 **   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **
 **                                                      **
 ** trace_printk() being used. Allocating extra memory.  **
 **                                                      **
 ** This means that this is a DEBUG kernel and it is     **
 ** unsafe for production use.                           **

Let's warn developers when they try to submit such a change.

Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
---

See also extensive discussion under this thread:
https://lkml.org/lkml/2020/8/20/244

This seems to be the simplest way to try to reduce the number
of trace_printk that make it into the kernel.

 scripts/checkpatch.pl | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4aa1d9d5e62c5b0..4b4988f25223c6a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4272,6 +4272,14 @@ sub process {
 			     "Prefer dev_$level(... to dev_printk(KERN_$orig, ...\n" . $herecurr);
 		}
 
+# trace_printk should not be used in production code.
+		if ($line =~ /\b(trace_printk|trace_puts|ftrace_vprintk)\s*\(/) {
+			my $func = $1;
+
+			WARN("TRACE_PRINTK",
+			     "Do not use $func() in production code (this can be ignored if built only behind a debug config option).\n" . $herecurr);
+		}
+
 # ENOSYS means "bad syscall nr" and nothing else.  This will have a small
 # number of false positives, but assembly files are not checked, so at
 # least the arch entry code will not trigger this warning.
-- 
2.28.0.297.g1956fa8f8d-goog

Re: [PATCH] checkpatch: Warn if trace_printk and friends are called

[Joe Perches]

On Tue, 2020-08-25 at 15:09 +0800, Nicolas Boichat wrote:
> trace_printk is meant as a debugging tool, and should not be
> compiled into production code without specific debug Kconfig
> options enabled, or source code changes, as indicated by the
> warning that shows up on boot if any trace_printk is called:
>  **   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **
>  **                                                      **
>  ** trace_printk() being used. Allocating extra memory.  **
>  **                                                      **
>  ** This means that this is a DEBUG kernel and it is     **
>  ** unsafe for production use.                           **
> 
> Let's warn developers when they try to submit such a change.
> 
> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
> ---
> 
> See also extensive discussion under this thread:
> https://lkml.org/lkml/2020/8/20/244
> 
> This seems to be the simplest way to try to reduce the number
> of trace_printk that make it into the kernel.

I don't have a real objection, just some nits.

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -4272,6 +4272,14 @@ sub process {
>  			     "Prefer dev_$level(... to dev_printk(KERN_$orig, ...\n" . $herecurr);
>  		}
>  
> +# trace_printk should not be used in production code.
> +		if ($line =~ /\b(trace_printk|trace_puts|ftrace_vprintk)\s*\(/) {
> +			my $func = $1;
> +
> +			WARN("TRACE_PRINTK",
> +			     "Do not use $func() in production code (this can be ignored if built only behind a debug config option).\n" . $herecurr);

You don't need to do an assign, you could use $1 directly.
I think 'built only behind' could be "building only with'.
No periods before the newline either.

> +		}
> +
>  # ENOSYS means "bad syscall nr" and nothing else.  This will have a small
>  # number of false positives, but assembly files are not checked, so at
>  # least the arch entry code will not trigger this warning.