Skip to content

Commit 600ae5c

Browse files
albankurtiintel-lab-lkp
authored andcommitted
checkpatch: add warning for pr_* and dev_* macros without a trailing newline
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 "f431c5c581fa1" Also, the patch is able to handle correctly if there is a printing call without a newline which then has a newline printed via pr_cont for both Rust and C alike. If there is no newline printed and the patch ends or there is another pr_* call before a newline with pr_cont is printed it will show a warning. Not implemented for dev_cont because it is not clear to me if that is used at all. One false warning that will be generated due to this change is in case we have a patch that modifies a `pr_* call without a newline` which has a pr_cont with a newline following it. In this case there will be a warning but because the patch does not include the following pr_cont it will warn there is nothing creating a newline. I have modified the warning to be softer due to this known problem. I have tested with comments, whitespace, differen orders of pr_* calls and pr_cont and the only case that I suspect to be a problem is the one outlined above. Suggested-by: Miguel Ojeda <[email protected]> Closes: Rust-for-Linux#1140 Signed-off-by: Alban Kurti <[email protected]>
1 parent ceff075 commit 600ae5c

File tree

1 file changed

+96
-0
lines changed

1 file changed

+96
-0
lines changed

scripts/checkpatch.pl

+96
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@
7777

7878
my %maybe_linker_symbol; # for externs in c exceptions, when seen in *vmlinux.lds.h
7979

80+
my $pending_log = undef;
81+
8082
sub help {
8183
my ($exitcode) = @_;
8284

@@ -3898,6 +3900,91 @@ sub process {
38983900
# check we are in a valid source file C or perl if not then ignore this hunk
38993901
next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/);
39003902

3903+
# check for pr_* and dev_* logs without a newline for C and Rust files to avoid missing log messages
3904+
my $pr_cont_pattern = qr{
3905+
\b
3906+
pr_cont!?
3907+
\s*
3908+
\(
3909+
\s*
3910+
"([^"]*)"
3911+
[^)]*
3912+
\)
3913+
}x;
3914+
my $log_macro_pattern = qr{
3915+
\b
3916+
(
3917+
pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
3918+
| dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
3919+
)
3920+
(!?)
3921+
\s*
3922+
\(
3923+
\s*
3924+
"([^"]*)"
3925+
}x;
3926+
3927+
if ($realfile =~ /\.(?:c|h|rs)$/) {
3928+
if ($rawline =~ /^\+/) {
3929+
my $cleanline = $rawline;
3930+
$cleanline =~ s/^[+\s]+//;
3931+
$cleanline =~ s/\r?$//;
3932+
$cleanline =~ s{/\*.*?\*/}{}g;
3933+
$cleanline =~ s{//.*}{}g;
3934+
3935+
if ($pending_log) {
3936+
if ($cleanline =~ /$pr_cont_pattern/) {
3937+
my $cont_string_arg = $1;
3938+
if ($cont_string_arg =~ /\\n$/) {
3939+
$pending_log = undef;
3940+
}
3941+
} elsif ($cleanline =~ /$log_macro_pattern/) {
3942+
WARN($pending_log->{lang} . "_LOG_NO_NEWLINE",
3943+
"Possible usage of $pending_log->{macro_call} without a trailing newline.\n" .
3944+
$pending_log->{herecurr});
3945+
3946+
$pending_log = undef;
3947+
3948+
my $macro_call = $1;
3949+
my $maybe_excl = $2;
3950+
my $string_arg = $3;
3951+
$string_arg =~ s/\s+$//;
3952+
3953+
if ($realfile =~ /\.rs$/ && $maybe_excl ne '!') {
3954+
return;
3955+
}
3956+
3957+
if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/) {
3958+
$pending_log = {
3959+
macro_call => $macro_call,
3960+
herecurr => $herecurr,
3961+
lang => ($realfile =~ /\.rs$/) ? "Rust" : "C",
3962+
};
3963+
}
3964+
}
3965+
} else {
3966+
if ($cleanline =~ /$log_macro_pattern/) {
3967+
my $macro_call = $1;
3968+
my $maybe_excl = $2;
3969+
my $string_arg = $3;
3970+
$string_arg =~ s/\s+$//;
3971+
3972+
if ($realfile =~ /\.rs$/ && $maybe_excl ne '!') {
3973+
return;
3974+
}
3975+
3976+
if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/) {
3977+
$pending_log = {
3978+
macro_call => $macro_call,
3979+
herecurr => $herecurr,
3980+
lang => ($realfile =~ /\.rs$/) ? "Rust" : "C",
3981+
};
3982+
}
3983+
}
3984+
}
3985+
}
3986+
}
3987+
39013988
# at the beginning of a line any tabs must come first and anything
39023989
# more than $tabsize must use tabs.
39033990
if ($rawline =~ /^\+\s* \t\s*\S/ ||
@@ -7678,6 +7765,15 @@ sub process {
76787765
}
76797766
}
76807767

7768+
# pending log means a pr_* without an ending newline has not
7769+
# been followed by a pr_cont call with a newline at the end
7770+
if ($pending_log) {
7771+
WARN($pending_log->{lang} . "_LOG_NO_NEWLINE",
7772+
"Usage of $pending_log->{macro_call} without a trailing newline.\n" .
7773+
$pending_log->{herecurr});
7774+
$pending_log = undef;
7775+
}
7776+
76817777
# If we have no input at all, then there is nothing to report on
76827778
# so just keep quiet.
76837779
if ($#rawlines == -1) {

0 commit comments

Comments
 (0)