Clean all the Clippy warnings in drivers/gpu/drm/drm_panic_qr.rs
#1123
Labels
• drivers
Related to the example drivers in `drivers/`.
easy
Expected to be an easy issue to resolve.
good first issue
Good for newcomers
Comments
ojeda
added
• drivers
|
I don't is specialist in clippy ... But , how I can run the clippy on the linux kernel without Cargo.toml ? |
|
All the Rust tools work without Cargo - |
|
I'm relatively new to kernel development so I'll leave this here if anyone is stumbling a little as well: Don't forget to enable @GuilhermeGiacomoSimoes Do you want to work on this? If not, I'd like to implement this and make my first contribution to the kernel! |
|
Okay @Witcher01 , you can make resolve this issue. |
|
I just sent the patches. Here's the thread: https://lore.kernel.org/rust-for-linux/[email protected]/T/#t Thanks for your help Miguel :) |
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this issue
Oct 12, 2024
Rust's standard library's `std::iter::Iterator` trait provides a function
`find` that finds the first element that satisfies a predicate.
The function `Version::from_segments` is doing the same thing but is
implementing the same logic itself.
Clippy complains about this in the `manual_find` lint:
error: manual implementation of `Iterator::find`
--> drivers/gpu/drm/drm_panic_qr.rs:212:9
|
212 | / for v in (1..=40).map(|k| Version(k)) {
213 | | if v.max_data() * 8 >= segments.iter().map(|s| s.total_size_bits(v)).sum() {
214 | | return Some(v);
215 | | }
216 | | }
217 | | None
| |____________^ help: replace with an iterator: `(1..=40).map(|k| Version(k)).find(|&v| v.max_data() * 8 >= segments.iter().map(|s| s.total_size_bits(v)).sum())`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_find
= note: `-D clippy::manual-find` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_find)]`
Use `Iterator::find` instead to make the intention clearer.
Reported-by: Miguel Ojeda <[email protected]>
Closes: Rust-for-Linux#1123
Signed-off-by: Thomas Böhler <[email protected]>
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this issue
Oct 12, 2024
The function `alignment_pattern` returns a static reference to a `u8`
slice. The borrow of the returned element in `ALIGNMENT_PATTERNS` is
already a reference as defined in the array definition above so this
borrow is unnecessary and removed by the compiler. Clippy notes this in
`needless_borrow`:
error: this expression creates a reference which is immediately dereferenced by the compiler
--> drivers/gpu/drm/drm_panic_qr.rs:245:9
|
245 | &ALIGNMENT_PATTERNS[self.0 - 1]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: change this to: `ALIGNMENT_PATTERNS[self.0 - 1]`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
= note: `-D clippy::needless-borrow` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::needless_borrow)]`
Remove the unnecessary borrow.
Reported-by: Miguel Ojeda <[email protected]>
Closes: Rust-for-Linux#1123
Signed-off-by: Thomas Böhler <[email protected]>
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this issue
Oct 12, 2024
Eliding lifetimes when possible instead of specifying them directly is
both shorter and easier to read. Clippy notes this in the
`needless_lifetimes` lint:
error: the following explicit lifetimes could be elided: 'b
--> drivers/gpu/drm/drm_panic_qr.rs:479:16
|
479 | fn new<'a, 'b>(segments: &[&Segment<'b>], data: &'a mut [u8]) -> Option<EncodedMsg<'a>> {
| ^^ ^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
= note: `-D clippy::needless-lifetimes` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::needless_lifetimes)]`
help: elide the lifetimes
|
479 - fn new<'a, 'b>(segments: &[&Segment<'b>], data: &'a mut [u8]) -> Option<EncodedMsg<'a>> {
479 + fn new<'a>(segments: &[&Segment<'_>], data: &'a mut [u8]) -> Option<EncodedMsg<'a>> {
|
Remove the explicit lifetime annotation in favour of an elided lifetime.
Reported-by: Miguel Ojeda <[email protected]>
Closes: Rust-for-Linux#1123
Signed-off-by: Thomas Böhler <[email protected]>
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this issue
Oct 12, 2024
Rust allows initializing fields of a struct without specifying the attribute that is assigned if the variable has the same name. In this instance this is done for all other attributes of the struct except for `data`. Remove the redundant `data` in the assignment to be consistent. Reported-by: Miguel Ojeda <[email protected]> Closes: Rust-for-Linux#1123 Signed-off-by: Thomas Böhler <[email protected]>
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this issue
Oct 12, 2024
It is common practice in Rust to indent the next line the same amount of
space as the previous one if both belong to the same list item. Clippy
checks for this with the lint `doc_lazy_continuation`.
error: doc list item without indentation
--> drivers/gpu/drm/drm_panic_qr.rs:979:5
|
979 | /// conversion to numeric segments.
| ^
|
= help: if this is supposed to be its own paragraph, add a blank line
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_lazy_continuation
= note: `-D clippy::doc-lazy-continuation` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::doc_lazy_continuation)]`
help: indent this line
|
979 | /// conversion to numeric segments.
| ++
Indent the offending line by 2 more spaces to remove this Clippy error.
Reported-by: Miguel Ojeda <[email protected]>
Closes: Rust-for-Linux#1123
Signed-off-by: Thomas Böhler <[email protected]>
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this issue
Oct 12, 2024
Clippy complains about a non-minimal boolean expression with
`nonminimal_bool`:
error: this boolean expression can be simplified
--> drivers/gpu/drm/drm_panic_qr.rs:722:9
|
722 | (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
= note: `-D clippy::nonminimal-bool` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]`
help: try
|
722 | !(x >= 8 || y >= 8 && y < end) || (x >= end && y < 8)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
722 | (y >= end || y < 8) && x < 8 || (x >= end && y < 8)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
While this can be useful in a lot of cases, it isn't here because the
line expresses clearly what the intention is. Simplifying the expression
means losing clarity, so opt-out of this lint for the offending line.
Reported-by: Miguel Ojeda <[email protected]>
Closes: Rust-for-Linux#1123
Signed-off-by: Thomas Böhler <[email protected]>
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this issue
Oct 12, 2024
Clippy warns about a reimplementation of `RangeInclusive::contains`:
error: manual `!RangeInclusive::contains` implementation
--> drivers/gpu/drm/drm_panic_qr.rs:986:8
|
986 | if version < 1 || version > 40 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!(1..=40).contains(&version)`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
= note: `-D clippy::manual-range-contains` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_range_contains)]`
Ignore this and keep the current implementation as that makes it easier
to read.
Reported-by: Miguel Ojeda <[email protected]>
Closes: Rust-for-Linux#1123
Signed-off-by: Thomas Böhler <[email protected]>
|
You're welcome! :) Let's see what the author and DRM maintainers say. Nicely written series, by the way. |
|
@Witcher01 Thanks for this series. If you want to test the QR code, without building the whole kernel, I've copied the rust code to a small project here: https://gitlab.com/kdj0c/qr_panic/ src/main.rs is almost identical to drm_panic_qr.rs, and you can test it with "cargo run", it's much easier to iterate, and see the result. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Clean all the Clippy warnings in
drivers/gpu/drm/drm_panic_qr.rs.The DRM maintainers will need to be involved, since the patch will go through them. For some warnings, there may be different ways of solving it, so it may require figuring out the best way for each for them.
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
Reported-by: tagand aCloses:tag to this issue. Please see https://rust-for-linux.com/contributing for details.Please take this issue only if you are new to the kernel development process and you would like to use it as a test to submit your first patch to the kernel. Please do not take it if you do not plan to make other contributions to the kernel.
The text was updated successfully, but these errors were encountered: