Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement checkpatch.pl check for pr_*! without \n #1140

Open
ojeda opened this issue Jan 19, 2025 · 1 comment
Open

Implement checkpatch.pl check for pr_*! without \n #1140

ojeda opened this issue Jan 19, 2025 · 1 comment
Labels
good first issue Good for newcomers medium Expected to be an issue of medium difficulty to resolve. • misc Related to other topics (e.g. CI).

Comments

@ojeda
Copy link
Member

ojeda commented Jan 19, 2025

Implement a checkpatch.pl check for pr_*! without \n (a warning, not an error, since one may be continuing the line), so that we reduce the chance of issues such as #1139.

Consider other printing facilities too that could be checked.

Consider if C could use such a check (if there is none yet).

This requires submitting a proper patch to the LKML and the Rust for Linux mailing list. Please recall to test your changes (including generating the documentation if changed, running the Rust doctests if changed, etc.), to use a proper title for the commit, to sign your commit under the Developer's Certificate of Origin and to add a Suggested-by: tag and a Link: tag to this issue. Please see https://docs.kernel.org/process/submitting-patches.html and https://rust-for-linux.com/contributing for details.

Please note that the checkpatch.pl maintainers will need to agree to the change and that the patch needs to be submitted to them.
@ojeda ojeda added good first issue Good for newcomers medium Expected to be an issue of medium difficulty to resolve. • misc Related to other topics (e.g. CI). labels Jan 19, 2025
@albankurti
Copy link

albankurti commented Feb 3, 2025 •
edited
Loading

Emailed potential implementation

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 7, 2025
@albankurti @intel-lab-lkp
checkpatch: add warning for pr_* and dev_* macros without a trailing …
600ae5c 
…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 <ojeda@kernel.org>
Closes: Rust-for-Linux#1140
Signed-off-by: Alban Kurti <kurti@invicto.ai>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 7, 2025
@albankurti @intel-lab-lkp
checkpatch: add warning for pr_* and dev_* macros without a trailing …
2e7993a 
…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 <ojeda@kernel.org>
Closes: Rust-for-Linux#1140
Signed-off-by: Alban Kurti <kurti@invicto.ai>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 11, 2025
@albankurti @akpm00
checkpatch: add warning for pr_* and dev_* macros without a trailing …
e954852 
…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.

Link: https://lkml.kernel.org/r/20250207-checkpatch-newline2-v4-1-26d8e80d0059@invicto.ai
Signed-off-by: Alban Kurti <kurti@invicto.ai>
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Closes: Rust-for-Linux#1140
Cc: Alex Gaynor <alex.gaynor@gmail.com>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Andreas Hindborg <a.hindborg@kernel.org>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Benno Lossin <benno.lossin@proton.me>
Cc: Björn Roy Baron <bjorn3_gh@protonmail.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
Cc: Gary Guo <gary@garyguo.net>
Cc: Joe Perches <joe@perches.com>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Trevor Gross <tmgross@umich.edu>
Cc: Charalampos Mitrodimas <charmitro@posteo.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 13, 2025
@albankurti @akpm00
checkpatch: add warning for pr_* and dev_* macros without a trailing …
460b6c2 
…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.

Link: https://lkml.kernel.org/r/20250207-checkpatch-newline2-v4-1-26d8e80d0059@invicto.ai
Signed-off-by: Alban Kurti <kurti@invicto.ai>
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Closes: Rust-for-Linux#1140
Cc: Alex Gaynor <alex.gaynor@gmail.com>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Andreas Hindborg <a.hindborg@kernel.org>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Benno Lossin <benno.lossin@proton.me>
Cc: Björn Roy Baron <bjorn3_gh@protonmail.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
Cc: Gary Guo <gary@garyguo.net>
Cc: Joe Perches <joe@perches.com>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Trevor Gross <tmgross@umich.edu>
Cc: Charalampos Mitrodimas <charmitro@posteo.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 14, 2025
@albankurti @akpm00
checkpatch: add warning for pr_* and dev_* macros without a trailing …
c9bd2d2 
…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.

Link: https://lkml.kernel.org/r/20250207-checkpatch-newline2-v4-1-26d8e80d0059@invicto.ai
Signed-off-by: Alban Kurti <kurti@invicto.ai>
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Closes: Rust-for-Linux#1140
Cc: Alex Gaynor <alex.gaynor@gmail.com>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Andreas Hindborg <a.hindborg@kernel.org>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Benno Lossin <benno.lossin@proton.me>
Cc: Björn Roy Baron <bjorn3_gh@protonmail.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
Cc: Gary Guo <gary@garyguo.net>
Cc: Joe Perches <joe@perches.com>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Trevor Gross <tmgross@umich.edu>
Cc: Charalampos Mitrodimas <charmitro@posteo.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers medium Expected to be an issue of medium difficulty to resolve. • misc Related to other topics (e.g. CI).
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ojeda @albankurti