-
Notifications
You must be signed in to change notification settings - Fork 456
Avoid unwrap()
in the kernel::list
examples
#1164
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
Comments
I can help with that Miguel :-) |
We try to leave the "easy"-tagged issues for those that have not sent any patch yet, so that they set up their email workflow etc., see above this note:
I think you have submitted one (was it pending a v3?), and I see a couple other commits in the kernel from you, but if you really want to do it and nobody else says anything in, say, a couple days, then please go ahead :) |
Yes, I will be submitting a v3 soon. I'm discussing the refinements with Benno and Danilo. I appreciate you keeping me in mind :-) . . . I'm not completely confident with other issues just yet. However, I'll look for one that isn't marked as "good first issue"to try, as I'm keen to keep contributing. Edit: Also, just to add to that, I'm currently participating in the Linux Foundation's Linux Kernel Bug Fixing Spring mentorship, so I'm really just starting out. If you happen to have any other issues in mind that you think might be a good fit for someone still learning, please don't hesitate to send them my way! :-) |
If you feel it would be useful for you to learn something doing this issue (e.g. on doctests), then please go ahead! :) By the way, other good first issues are fine to pick -- what I meant is to try to avoid the trivial ones or the ones you feel you wouldn't learn much from them, so that others have a chance to solve them too. Thanks! |
Hey, I’m new to kernel development process and I’d love to try this out! |
Nice @albus-droid go ahead !! Thanks =) TMJ 🇧🇷 |
I replaced -/// assert_eq!(items.next().unwrap().value, 14);
+/// assert_eq!(items.next().ok_or(EINVAL)?.value, 14); is this the right approach? |
Replace panicking `unwrap()` calls in the `kernel::list` doctests with `ok_or(EINVAL)?` so they return a proper `Error` instead of panicking. Suggested-by: Miguel Ojeda <[email protected]> Link: Rust-for-Linux#1164 Signed-off-by: Albin Babu Varghese <[email protected]>
Replace panicking `unwrap()` calls in the `kernel::list` doctests with `ok_or(EINVAL)?` so they return a proper `Error` instead of panicking. Suggested-by: Miguel Ojeda <[email protected]> Link: Rust-for-Linux#1164 Signed-off-by: Albin Babu Varghese <[email protected]>
Using `unwrap()` in kernel doctests can cause panics on error and may give newcomers the mistaken impression that panicking is acceptable in kernel code. Replace all `.unwrap()` calls in `kernel::list` examples with `.ok_or(EINVAL)?` so that errors are properly propagated. Closes: Rust-for-Linux#1164 Suggested-by: Miguel Ojeda <[email protected]> Reviewed-by: Benno Lossin <[email protected]> Link: Rust-for-Linux#1164 Signed-off-by: Albin Babu Varghese <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.
Clean the
kernel::list
examples so that they avoidunwrap()
, which panics.Instead, show users how to use the
?
operator, wherever possible.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 aLink:
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: