-
Notifications
You must be signed in to change notification settings - Fork 138
refactor(ironrdp-web): fix indexing_slicing
clippy lint warnings
#993
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.
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
base: master
Are you sure you want to change the base?
Conversation
Coverage Report 🤖 ⚙️Past: New: Diff: -0.02% [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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preciously this was subject to panics upon bad input?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
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: