Cleanup Clippy issues in drm_panic_qr.rs
Subsystems (3)
v2 cover Cleanup Clippy issues in drm_panic_qr.rs
The file drivers/gpu/drm/drm_panic_qr.rs has some lints that Clippy complains about. This series cleans them up by either allowing what is written or conforming to what Clippy expects where it makes sense.
All explicitly allowed lints are marked with #[expect(...)].
v1 -> v2:
- Add "Fixes" trailers and rename "Closes" to "Link" trailers as the patches all fix part of the issue.
- Replace #[allow(...)] with #[expect(...)]. Support for expect is already in rust-next, which is where this series will be merged into.
Thomas Böhler (7):
drm/panic: avoid reimplementing Iterator::find drm/panic: remove unnecessary borrow in alignment_pattern drm/panic: prefer eliding lifetimes
drm/panic: remove redundant field when assigning value drm/panic: correctly indent continuation of line in list item drm/panic: allow verbose boolean for clarity drm/panic: allow verbose version check
drivers/gpu/drm/drm_panic_qr.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
Signature · 1 line
2.46.2
PATCH v2 1/7 drm/panic: avoid reimplementing Iterator::find
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.
--- drivers/gpu/drm/drm_panic_qr.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/gpu/drm/drm_panic_qr.rs +3 -6
@@ -209,12 +209,9 @@impl Version {/// Returns the smallest QR version than can hold these segments. fn from_segments (segments: &[&Segment <'_>]) ->Option <Version > { -for vin (1 ..=40 ).map (|k|Version (k)) { -if v.max_data () *8 >= segments.iter ().map (|s| s.total_size_bits (v)).sum () { -return Some (v); - } - } -None + (1 ..=40 ) + .map (Version ) + .find (|&v| v.max_data () *8 >= segments.iter ().map (|s| s.total_size_bits (v)).sum ()) }fn width (&self) ->u8 {
Signature · 1 line
2.46.2
PATCH v2 2/7 drm/panic: remove unnecessary borrow in alignment_pattern
In reply to Thomas BöhlerThe 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.
--- drivers/gpu/drm/drm_panic_qr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/drm_panic_qr.rs +1 -1
@@ -239,7 +239,7 @@ fn g1_blk_size(&self) -> usize { }fn alignment_pattern (&self) -> &'static [u8 ] { - &ALIGNMENT_PATTERNS [self.0 -1 ] +ALIGNMENT_PATTERNS [self.0 -1 ] }fn poly (&self) -> &'static [u8 ] {
Signature · 1 line
2.46.2
PATCH v2 3/7 drm/panic: prefer eliding lifetimes
In reply to Thomas BöhlerEliding 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.
--- drivers/gpu/drm/drm_panic_qr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/drm_panic_qr.rs +1 -1
@@ -476,7 +476,7 @@ struct EncodedMsg<'a> {/// Data to be put in the QR code, with correct segment encoding, padding, and /// Error Code Correction. impl EncodedMsg <'_> { -fn new <'a, 'b>(segments: &[&Segment <'b>], data: &'amut [u8 ]) ->Option <EncodedMsg <'a>> { +fn new <'a>(segments: &[&Segment <'_>], data: &'amut [u8 ]) ->Option <EncodedMsg <'a>> {let version =Version ::from_segments (segments)?;let ec_size = version.ec_size ();let g1_blocks = version.g1_blocks ();
Signature · 1 line
2.46.2
PATCH v2 4/7 drm/panic: remove redundant field when assigning value
In reply to Thomas BöhlerRust 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.
--- drivers/gpu/drm/drm_panic_qr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/drm_panic_qr.rs +1 -1
@@ -489,7 +489,7 @@ fn new<'a>(segments: &[&Segment<'_>], data: &'a mut [u8]) -> Option<EncodedMsg<' data.fill (0 );let mut em =EncodedMsg { - data: data, + data, ec_size, g1_blocks, g2_blocks,
Signature · 1 line
2.46.2
PATCH v2 5/7 drm/panic: correctly indent continuation of line in list item
In reply to Thomas BöhlerIt 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.
--- drivers/gpu/drm/drm_panic_qr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/drm_panic_qr.rs +1 -1
@@ -976,7 +976,7 @@ fn draw_all(&mut self, data: impl Iterator<Item = u8>) {/// * `url_len`: Length of the URL. /// /// * If `url_len` > 0, remove the 2 segments header/length and also count the -/// conversion to numeric segments. +/// conversion to numeric segments. /// * If `url_len` = 0, only removes 3 bytes for 1 binary segment. #[no_mangle] pub extern "C" fn drm_panic_qr_max_data_size (version:u8 , url_len:usize ) ->usize {
Signature · 1 line
2.46.2
PATCH v2 6/7 drm/panic: allow verbose boolean for clarity
In reply to Thomas BöhlerClippy 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.
---
v1 -> v2: turn the introduced explicit "return" statement into a block
drivers/gpu/drm/drm_panic_qr.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/gpu/drm/drm_panic_qr.rs +4 -1
@@ -719,7 +719,10 @@ fn draw_finders(&mut self) {fn is_finder (&self, x:u8 , y:u8 ) ->bool {let end = self.width -8 ; - (x <8 && y <8 ) || (x <8 && y >= end) || (x >= end && y <8 ) +#[expect(clippy::nonminimal_bool)] + { + (x <8 && y <8 ) || (x <8 && y >= end) || (x >= end && y <8 ) + } }// Alignment pattern: 5x5 squares in a grid.
Signature · 1 line
2.46.2
PATCH v2 7/7 drm/panic: allow verbose version check
In reply to Thomas BöhlerClippy 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.
--- drivers/gpu/drm/drm_panic_qr.rs | 1 + 1 file changed, 1 insertion(+)
drivers/gpu/drm/drm_panic_qr.rs +1 -0
@@ -983,6 +983,7 @@ fn draw_all(&mut self, data: impl Iterator<Item = u8>) {/// * If `url_len` = 0, only removes 3 bytes for 1 binary segment. #[no_mangle] pub extern "C" fn drm_panic_qr_max_data_size (version:u8 , url_len:usize ) ->usize { +#[expect(clippy::manual_range_contains)] if version <1 || version >40 {return 0 ; }
Signature · 1 line
2.46.2
Miguel Ojeda
AppliedQuoted 4 lines · Thomas Böhler
The file
drivers/gpu/drm/drm_panic_qr.rshas some lints that Clippy complains about. This series cleans them up by either allowing what is written or conforming to what Clippy expects where it makes sense.
Applied to rust-next -- thanks everyone!
[ Reworded to indent Clippy's message. - Miguel ]
[ Reworded to add Clippy warning like it is done in the rest of the
series. - Miguel ]
[ Reworded to mention the redundant closure cleanup too. - Miguel ]
Cheers,
Miguel