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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > Closes: https://github.com/Rust-for-Linux/linux/issues/1140
> > Signed-off-by: Alban Kurti <kurti@xxxxxxxxxx>
> > ---
> > 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