refactor(ironrdp-web): fix indexing_slicing clippy lint warnings#993
refactor(ironrdp-web): fix indexing_slicing clippy lint warnings#993RRRadicalEdward wants to merge 5 commits intomasterfrom
indexing_slicing clippy lint warnings#993Conversation
Coverage Report 🤖 ⚙️Past: New: Diff: +0.00% [this comment will be updated automatically] |
crates/ironrdp-web/src/canvas.rs
Outdated
| let mut src = buffer.chunks_exact(4).map(|pixel| { | ||
| let r = pixel[0]; | ||
| let g = pixel[1]; | ||
| let b = pixel[2]; | ||
| let r = *pixel.first().expect("index cannot be out of bounds"); | ||
| let g = *pixel.get(1).expect("index cannot be out of bounds"); | ||
| let b = *pixel.get(2).expect("index cannot be out of bounds"); |
There was a problem hiding this comment.
Looks a bit too verbose, although there may be a better way to write this with the new stabilized APIs
There was a problem hiding this comment.
Do you have https://doc.rust-lang.org/std/primitive.slice.html#method.first_chunk in mind or something different? The perfect API here would be - https://doc.rust-lang.org/std/primitive.slice.html#method.as_array, but it's nightly only
| let src_slice = src.get(src_begin..src_end).with_context(|| { | ||
| format!( | ||
| "invalid region {region:?} for image with dimensions {}x{}", | ||
| image.width(), | ||
| image.height() | ||
| ) | ||
| })?; |
There was a problem hiding this comment.
Preciously this was subject to panics upon bad input?
There was a problem hiding this comment.
Yeah. I think it was possible to trigger panic if passing an empty image and any valid region.
|
@CBenoit Does it look good? Should we proceed with adding this lint? |
|
I want to double check a little bit more before merging this. |
|
I’m putting back this PR on my TODO review list. |
Hi @CBenoit. Did you have a chance to double-check this PR? |
3 participants
This lint
This one is quite pedantic and changes the way we write the code, but on the other hand, it makes the code more panic-resistant. Fixed the lint warnings only for ironrdp-web to see early feedback on it.
If we agree on adding it, I will add this lint for each crate separately, because there are too many warnings to do in 1 PR: