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

[Charalampos Mitrodimas <charmitro@posteo.net>]

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