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

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

[Alban Kurti]

Fixes the false positive warning when there is a comment on a pr_ or
dev_ log that ends with \n correctly.
Also contains some improvements on the macro_pattern to make it more
concise.

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Closes: https://github.com/Rust-for-Linux/linux/issues/1140
Signed-off-by: Alban Kurti <kurti@invicto.ai>
---
Changelog since v1:
 - Strip comments before matching to avoid false positives.
 - Refactored the macro patterns for clarity.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9eed3683ad76..3256b5f31835 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3888,6 +3888,52 @@ sub process {
 			}
 		}
 
+# check for pr_* and dev_* logs without a newline for C and Rust files to avoid missing log messages
+    my $log_macro_pattern = qr{
+        \b
+        (
+            pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
+          | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
+        )
+        (!?)
+        \s*
+        \(
+        \s*
+        "([^"]*)"
+    }x;
+
+    if ($realfile =~ /\.(?:c|h|rs)$/) {
+        if ($rawline =~ /^\+/) {
+            my $cleanline = $rawline;
+
+            $cleanline =~ s/^[+\s]+//;
+            $cleanline =~ s/\r?$//;
+
+            $cleanline =~ s{/\*.*?\*/}{}g;
+            $cleanline =~ s{//.*}{}g;
+
+            if ($cleanline =~ /$log_macro_pattern/) {
+                my $macro_call = $1;
+                my $maybe_excl  = $2;
+                my $string_arg  = $3;
+
+                $string_arg =~ s/\s+$//;
+                if ($realfile =~ /\.rs$/ && $maybe_excl ne '!') {
+                    return;
+                }
+
+                if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/) {
+                    my $lang = ($realfile =~ /\.rs$/) ? "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

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

[Alban Kurti]

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>
Closes: https://github.com/Rust-for-Linux/linux/issues/1140
Signed-off-by: Alban Kurti <kurti@invicto.ai>
---
Changelog since v1:
 - Strip comments before matching to avoid false positives.
 - Refactored the macro patterns for clarity.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9eed3683ad76..3256b5f31835 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3888,6 +3888,52 @@ sub process {
 			}
 		}
 
+# check for pr_* and dev_* logs without a newline for C and Rust files to avoid missing log messages
+    my $log_macro_pattern = qr{
+        \b
+        (
+            pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
+          | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
+        )
+        (!?)
+        \s*
+        \(
+        \s*
+        "([^"]*)"
+    }x;
+
+    if ($realfile =~ /\.(?:c|h|rs)$/) {
+        if ($rawline =~ /^\+/) {
+            my $cleanline = $rawline;
+
+            $cleanline =~ s/^[+\s]+//;
+            $cleanline =~ s/\r?$//;
+
+            $cleanline =~ s{/\*.*?\*/}{}g;
+            $cleanline =~ s{//.*}{}g;
+
+            if ($cleanline =~ /$log_macro_pattern/) {
+                my $macro_call = $1;
+                my $maybe_excl  = $2;
+                my $string_arg  = $3;
+
+                $string_arg =~ s/\s+$//;
+                if ($realfile =~ /\.rs$/ && $maybe_excl ne '!') {
+                    return;
+                }
+
+                if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/) {
+                    my $lang = ($realfile =~ /\.rs$/) ? "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

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

[Alban Kurti]

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>
Closes: https://github.com/Rust-for-Linux/linux/issues/1140
Signed-off-by: Alban Kurti <kurti@invicto.ai>
---
Changelog since v1:
 - Strip comments before matching to avoid false positives.
 - Refactored the macro patterns for clarity.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9eed3683ad76..3256b5f31835 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3888,6 +3888,52 @@ sub process {
 			}
 		}
 
+# check for pr_* and dev_* logs without a newline for C and Rust files to avoid missing log messages
+    my $log_macro_pattern = qr{
+        \b
+        (
+            pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
+          | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
+        )
+        (!?)
+        \s*
+        \(
+        \s*
+        "([^"]*)"
+    }x;
+
+    if ($realfile =~ /\.(?:c|h|rs)$/) {
+        if ($rawline =~ /^\+/) {
+            my $cleanline = $rawline;
+
+            $cleanline =~ s/^[+\s]+//;
+            $cleanline =~ s/\r?$//;
+
+            $cleanline =~ s{/\*.*?\*/}{}g;
+            $cleanline =~ s{//.*}{}g;
+
+            if ($cleanline =~ /$log_macro_pattern/) {
+                my $macro_call = $1;
+                my $maybe_excl  = $2;
+                my $string_arg  = $3;
+
+                $string_arg =~ s/\s+$//;
+                if ($realfile =~ /\.rs$/ && $maybe_excl ne '!') {
+                    return;
+                }
+
+                if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/) {
+                    my $lang = ($realfile =~ /\.rs$/) ? "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 v2 RESEND 1/1] checkpatch: add warning for pr_* and dev_* macros without a trailing newline

[Charalampos Mitrodimas]

Alban Kurti <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>
> Closes: https://github.com/Rust-for-Linux/linux/issues/1140
> Signed-off-by: Alban Kurti <kurti@invicto.ai>
> ---
> Changelog since v1:
>  - Strip comments before matching to avoid false positives.
>  - Refactored the macro patterns for clarity.
>
>  scripts/checkpatch.pl | 46 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 9eed3683ad76..3256b5f31835 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3888,6 +3888,52 @@ sub process {
>  			}
>  		}
>  
> +# check for pr_* and dev_* logs without a newline for C and Rust files to avoid missing log messages
> +    my $log_macro_pattern = qr{
> +        \b
> +        (
> +            pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
> +          | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
> +        )
> +        (!?)
> +        \s*
> +        \(
> +        \s*
> +        "([^"]*)"
> +    }x;
> +
> +    if ($realfile =~ /\.(?:c|h|rs)$/) {
> +        if ($rawline =~ /^\+/) {
> +            my $cleanline = $rawline;
> +
> +            $cleanline =~ s/^[+\s]+//;
> +            $cleanline =~ s/\r?$//;
> +
> +            $cleanline =~ s{/\*.*?\*/}{}g;
> +            $cleanline =~ s{//.*}{}g;
> +
> +            if ($cleanline =~ /$log_macro_pattern/) {
> +                my $macro_call = $1;
> +                my $maybe_excl  = $2;
> +                my $string_arg  = $3;
> +
> +                $string_arg =~ s/\s+$//;
> +                if ($realfile =~ /\.rs$/ && $maybe_excl ne '!') {
> +                    return;
> +                }
> +
> +                if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/) {
> +                    my $lang = ($realfile =~ /\.rs$/) ? "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/) {

I couldn't apply this to my tree (with rust-next base), but manually
adding the changes to scripts/checkpatch.pl works as expected.

Alban, can you try resetting your tree to rust-next and apply this
patch? I believe (not sure) there are some indentation issues with the
code.

C. Mitrodimas

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

[Alban Kurti]

Hi Charalampos,

Thank you for testing.

I just send v4, ignore v3 please. It should fix the formatting issue,
additionally, I thought of implementing a new feature further as I
think it can enhance the functionality, but I can always revert to the
v2 approach if it is deemed to complicated or unnecessary.

Kind regards,
Alban Kurti

On Fri, 2025-02-07 at 18:19 +0000, Charalampos Mitrodimas wrote:
> Alban Kurti <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>
> > Closes: https://github.com/Rust-for-Linux/linux/issues/1140
> > Signed-off-by: Alban Kurti <kurti@invicto.ai>
> > ---
> > Changelog since v1:
> >  - Strip comments before matching to avoid false positives.
> >  - Refactored the macro patterns for clarity.
> > 
> >  scripts/checkpatch.pl | 46
> > +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 9eed3683ad76..3256b5f31835 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -3888,6 +3888,52 @@ sub process {
> >  			}
> >  		}
> >  
> > +# check for pr_* and dev_* logs without a newline for C and Rust
> > files to avoid missing log messages
> > +    my $log_macro_pattern = qr{
> > +        \b
> > +        (
> > +            pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
> > +          | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
> > +        )
> > +        (!?)
> > +        \s*
> > +        \(
> > +        \s*
> > +        "([^"]*)"
> > +    }x;
> > +
> > +    if ($realfile =~ /\.(?:c|h|rs)$/) {
> > +        if ($rawline =~ /^\+/) {
> > +            my $cleanline = $rawline;
> > +
> > +            $cleanline =~ s/^[+\s]+//;
> > +            $cleanline =~ s/\r?$//;
> > +
> > +            $cleanline =~ s{/\*.*?\*/}{}g;
> > +            $cleanline =~ s{//.*}{}g;
> > +
> > +            if ($cleanline =~ /$log_macro_pattern/) {
> > +                my $macro_call = $1;
> > +                my $maybe_excl  = $2;
> > +                my $string_arg  = $3;
> > +
> > +                $string_arg =~ s/\s+$//;
> > +                if ($realfile =~ /\.rs$/ && $maybe_excl ne '!') {
> > +                    return;
> > +                }
> > +
> > +                if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/)
> > {
> > +                    my $lang = ($realfile =~ /\.rs$/) ? "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/) {
> 
> I couldn't apply this to my tree (with rust-next base), but manually
> adding the changes to scripts/checkpatch.pl works as expected.
> 
> Alban, can you try resetting your tree to rust-next and apply this
> patch? I believe (not sure) there are some indentation issues with
> the
> code.
> 
> C. Mitrodimas