* [PATCH v2 RESEND 1/1] checkpatch: add warning for pr_* and dev_* macros without a trailing newline
@ 2025-02-06 12:09 Alban Kurti
2025-02-07 18:19 ` Charalampos Mitrodimas
0 siblings, 1 reply; 5+ messages in thread
From: Alban Kurti @ 2025-02-06 12:09 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Charalampos Mitrodimas, Mugel Ojeda, Alban Kurti,
linux-kernel, rust-for-linux
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 RESEND 1/1] checkpatch: add warning for pr_* and dev_* macros without a trailing newline
2025-02-06 12:09 [PATCH v2 RESEND 1/1] checkpatch: add warning for pr_* and dev_* macros without a trailing newline Alban Kurti
@ 2025-02-07 18:19 ` Charalampos Mitrodimas
2025-02-07 18:40 ` Alban Kurti
0 siblings, 1 reply; 5+ messages in thread
From: Charalampos Mitrodimas @ 2025-02-07 18:19 UTC (permalink / raw)
To: Alban Kurti
Cc: Joe Perches, Andrew Morton, Mugel Ojeda, linux-kernel,
rust-for-linux
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2 RESEND 1/1] checkpatch: add warning for pr_* and dev_* macros without a trailing newline
2025-02-07 18:19 ` Charalampos Mitrodimas
@ 2025-02-07 18:40 ` Alban Kurti
0 siblings, 0 replies; 5+ messages in thread
From: Alban Kurti @ 2025-02-07 18:40 UTC (permalink / raw)
To: Charalampos Mitrodimas
Cc: Joe Perches, Andrew Morton, Mugel Ojeda, linux-kernel,
rust-for-linux
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 RESEND 1/1] checkpatch: add warning for pr_* and dev_* macros without a trailing newline
@ 2025-02-06 12:04 Alban Kurti
0 siblings, 0 replies; 5+ messages in thread
From: Alban Kurti @ 2025-02-06 12:04 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Charalampos Mitrodimas, Mugel Ojeda, Alban Kurti,
linux-kernel, rust-for-linux
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 RESEND 1/1] checkpatch: add warning for pr_* and dev_* macros without a trailing newline
@ 2025-02-06 12:03 Alban Kurti
0 siblings, 0 replies; 5+ messages in thread
From: Alban Kurti @ 2025-02-06 12:03 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Charalampos Mitrodimas, Mugel Ojeda, Alban Kurti,
linux-kernel, rust-for-linux
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-02-07 18:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-06 12:09 [PATCH v2 RESEND 1/1] checkpatch: add warning for pr_* and dev_* macros without a trailing newline Alban Kurti
2025-02-07 18:19 ` Charalampos Mitrodimas
2025-02-07 18:40 ` Alban Kurti
-- strict thread matches above, loose matches on Subject: below --
2025-02-06 12:04 Alban Kurti
2025-02-06 12:03 Alban Kurti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox