Skip to content
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

parsing: remove potential panics #28

Closed
wants to merge 1 commit into from

Conversation

rmsyn
Copy link
Contributor

@rmsyn rmsyn commented Apr 22, 2024

Removes the unwrap calls in BigEndian*::from_bytes implementations.

Currently, the implementation will never panic on the unwrap call, because the prior get call always returns the appropriate length byte slice.

However, it is safer to call .ok()? to bubble-up the None result. Which, again, will never be returned with the current implementation. The function will bubble-up None on the get call before reaching the try_into.

Removes the `unwrap` calls in `BigEndian*::from_bytes` implementations.

Currently, the implementation will never panic on the `unwrap` call,
because the prior `get` call always returns the appropriate length byte
slice.

However, it is safer to call `.ok()?` to bubble-up the `None` result.
Which, again, will never be returned with the current implementation.
The function will bubble-up `None` on the `get` call before reaching the
`try_into`.
@repnop
Copy link
Owner

repnop commented May 18, 2024

I think I'm going to close this as it better describes the intent that this will never panic, and at opt-level = 3 they both produce identical results, thanks for the PR though!

@repnop repnop closed this May 18, 2024
@rmsyn
Copy link
Contributor Author

rmsyn commented May 19, 2024

I think I'm going to close this as it better describes the intent that this will never panic

I have reviewed and used a number of libraries that make this same assumption to disastrous results.

There are enough places where implicit panics can crash Rust programs. Removing and safe-guarding against panics, making panic-safe Rust, is a very important part of hardening Rust software. This is especially true of no_std software that is likely being used in kernels, firmware, and/or bare-metal programs.

Your project though, so best of luck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants