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

Some unsafe code is not marked as such #1

Open
whitequark opened this issue Apr 29, 2018 · 2 comments
Open

Some unsafe code is not marked as such #1

whitequark opened this issue Apr 29, 2018 · 2 comments

Comments

@whitequark
Copy link

For example, calling EthernetDevice::new twice results in undefined behavior, but it's not unsafe.

@adamgreig
Copy link
Member

EthernetDevice::new probably could do with being marked unsafe, but maybe the better solution here is to make a single &'static mut EthernetDevice with an unsafe init function instead, since it's never sensible to make more than one of them.

For an end application like this I wonder how far up the stack to push unsafe.

Many functions in bootload and flash are unsafe if constants in config.rs are invalid and OK otherwise; they could check the values from config seem reasonable (i.e., inside valid memory regions so as to not segfault), but they can't check that they're actually correct. I'm not sure anything else is egregiously unsafe. Probably bootload::bootload and bootload::reset_bootload should be marked divergent, though.

@whitequark
Copy link
Author

I'm not sure anything else is egregiously unsafe.

Quite a few functions I think. For example, TDes::set_length and RDes::release can cause unsafety if not used correctly as they do not themselves enforce the required invariants (even if they're not public).

For an end application like this I wonder how far up the stack to push unsafe.

All the way up! An illusion of safety is much worse than most functions being marked unsafe. The latter is just C but the former makes it all but impossible to trust any part of the code that is ostensibly safe.

adamgreig referenced this issue in adamgreig/blethrs Aug 8, 2018
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

No branches or pull requests

2 participants