[PATCH 1/1 RESEND] checkpatch: add warning for pr_* and dev_* macros without a trailing newline

[PATCH 1/1 RESEND] checkpatch: add warning for pr_* and dev_* macros without a trailing newline

[albankurti]

Add a new check in scripts/checkpatch.pl to detect usage of pr_(level)
and dev_(level) macros (for both C and Rust) when the string literal
does not end with '\n'. Missing trailing newlines can lead to incomplete
log lines that do not appear properly in dmesg or in console output.
To show an example of this working after applying the patch we can run
the script on the commit that likely motivated this need/issue:
  ./scripts/checkpatch.pl --strict -g f431c5c581fa176f608ba3fdebb3c1051bad5774

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/1140
Signed-off-by: albankurti <kurti@invicto.ai>
---
 scripts/checkpatch.pl | 48 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9eed3683ad76..86ff666f5017 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3888,6 +3888,54 @@ sub process {
 			}
 		}
 
+# check for pr_* and dev_* logs without a newline for C and Rust files to avoid missing log messages
+    my $macro_pattern_c = qr{
+        \b
+        (
+            pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
+          | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
+        )
+        \(\s*
+        "((?:\\.|[^"])*)"
+        \s*
+        (?:,|\))
+    }x;
+
+    my $macro_pattern_rust = qr{
+        \b
+        (
+            pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
+          | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
+        )
+        !\s*
+        \(\s*
+        "((?:\\.|[^"])*)"
+        \s*
+        (?:,|\))
+    }x;
+
+    if ($realfile =~ /\.(?:c|h|rs)$/) {
+      my $is_rust = ($realfile =~ /\.rs$/);
+      my $macro_pattern = $is_rust ? $macro_pattern_rust : $macro_pattern_c;
+
+      if ($line =~ /^\+/) {
+        my $cleanline = $line;
+        $cleanline =~ s/^[+\s]+//;
+        $cleanline =~ s/\r?$//;
+
+        if ($cleanline =~ /$macro_pattern/) {
+          my $macro_call = $1;
+          my $string_arg = $2;
+
+          if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/) {
+            my $lang = $is_rust ? "Rust" : "C";
+            WARN("${lang}_LOG_NO_NEWLINE",
+                 "Usage of $macro_call without a trailing newline. Consider adding '\\n'.\n" . $herecurr);
+          }
+        }
+      }
+    }
+
 # check for .L prefix local symbols in .S files
 		if ($realfile =~ /\.S$/ &&
 		    $line =~ /^\+\s*(?:[A-Z]+_)?SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*\(\s*\.L/) {
-- 
2.48.1

Re: [PATCH 1/1 RESEND] checkpatch: add warning for pr_* and dev_* macros without a trailing newline

[Charalampos Mitrodimas]

albankurti <kurti@invicto.ai> writes:

> Add a new check in scripts/checkpatch.pl to detect usage of pr_(level)
> and dev_(level) macros (for both C and Rust) when the string literal
> does not end with '\n'. Missing trailing newlines can lead to incomplete
> log lines that do not appear properly in dmesg or in console output.
> To show an example of this working after applying the patch we can run
> the script on the commit that likely motivated this need/issue:
>   ./scripts/checkpatch.pl --strict -g f431c5c581fa176f608ba3fdebb3c1051bad5774
>
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://github.com/Rust-for-Linux/linux/issues/1140
> Signed-off-by: albankurti <kurti@invicto.ai>
> ---
>  scripts/checkpatch.pl | 48 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 9eed3683ad76..86ff666f5017 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3888,6 +3888,54 @@ sub process {
>  			}
>  		}
>  
> +# check for pr_* and dev_* logs without a newline for C and Rust files to avoid missing log messages
> +    my $macro_pattern_c = qr{
> +        \b
> +        (
> +            pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
> +          | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
> +        )
> +        \(\s*
> +        "((?:\\.|[^"])*)"
> +        \s*
> +        (?:,|\))
> +    }x;
> +
> +    my $macro_pattern_rust = qr{
> +        \b
> +        (
> +            pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
> +          | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
> +        )
> +        !\s*
> +        \(\s*
> +        "((?:\\.|[^"])*)"
> +        \s*
> +        (?:,|\))
> +    }x;
> +
> +    if ($realfile =~ /\.(?:c|h|rs)$/) {
> +      my $is_rust = ($realfile =~ /\.rs$/);
> +      my $macro_pattern = $is_rust ? $macro_pattern_rust : $macro_pattern_c;
> +
> +      if ($line =~ /^\+/) {
> +        my $cleanline = $line;
> +        $cleanline =~ s/^[+\s]+//;
> +        $cleanline =~ s/\r?$//;
> +
> +        if ($cleanline =~ /$macro_pattern/) {
> +          my $macro_call = $1;
> +          my $string_arg = $2;
> +
> +          if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/) {
> +            my $lang = $is_rust ? "Rust" : "C";
> +            WARN("${lang}_LOG_NO_NEWLINE",
> +                 "Usage of $macro_call without a trailing newline. Consider adding '\\n'.\n" . $herecurr);
> +          }

Having a comment breaks this,

  $ ./scripts/checkpatch.pl --strict -f newlines.rs
  [...]
  WARNING: Usage of pr_err without a trailing newline. Consider adding '\n'.
  #3: FILE: newlines.rs:3:
  +pr_err!("Has newline\n"); // A comment

I believe you also have to strip out comments?

> +        }
> +      }
> +    }
> +
>  # check for .L prefix local symbols in .S files
>  		if ($realfile =~ /\.S$/ &&
>  		    $line =~ /^\+\s*(?:[A-Z]+_)?SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*\(\s*\.L/) {