-
Notifications
You must be signed in to change notification settings - Fork 44
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
chore: remove all unsafe #169
Conversation
@nspin I would really appreciate your opinion on this |
1c57f85
to
382c293
Compare
Is this fixing a known bug? The biggest motivation for re-writing the tool from Python to Rust was to avoid dependencies. Because of how few |
None that I'm aware of. Manual conversion byte by byte is fine! There is one occurrence in the code where a mutable ref to a struct is mutated, thus in-place editing the byte slice, that part may need refactoring a little. Doing it by hand will add some more code for sure. It might be sensible to use a macro_rules macro to generate both the to_bytes/from_bytes functions. It of course is the way for the least amount of I understand the general wariness towards dependencies, however, if one where to compare different libs, then Avoiding dependencies sounds like a requirement which itself is traceable to other requirement(s). Is there any documentation/public discussion about this? I'd like to understand the rationale behind the requirement. |
This commit removes all unsafe (entirely related to transmuting) from the code. Instead, Google's zerocopy crate is used to do the transmuting. Being widely used and thoroughly vetted, it is less likely to cause hidden soundness issues. Signed-off-by: wucke13 <[email protected]>
382c293
to
2e7ffdc
Compare
Just my 2 cents, but my personal experience with this kind of dependency (esp from Google in the Java ecosystem) has been abysmal. They can replace things you can easily do yourself with opaque complex code that does much more than you need and you now have the problem of constantly updating those dependencies when something that might be unrelated to your code is getting fixed upstream. I'm not saying using libraries is bad, but the tradeoffs really need to be considered deeply and case by case. E.g. if we were doing lots of networking stuff here, I think the library would be great. For these controlled simple cases, probably less so. (I also do agree with reducing |
I agree with what Gerwin has said. Irrespective of the programming language, I find dependencies in general quite dangerous for the longevity and reliability of software, which means we have to evaluate each one we want to add on a case by case basis. In this case, I would say to replace all the unsafe with manually converting bytes would be 100-200 lines of code. The code itself would be fairly straightfoward I imagine. The An example where it is worthwhile is the current dependencies of the Microkit tool (an XML parser, and a JSON parser). These would be non-trivial to write ourselves and would most likely have more correctness issues than the dependencies. I have to push back on adding more dependencies, otherwise I fear the situation will quickly get out of hand.
There is not to my knowledge. I don't think the seL4 Foundation has anything official, but I could add something about the principles of Microkit in #49. It would be good to have something to point to in the future if this topic comes up again. |
Thank you two for the elaborate explanations! This is quite funny, on the one hand I'm bitten exactly by what you describe with the Nix packaging of the On the other hand, in my past fiver-or-so years of Rust, this was seldomly a pain. Of course there are poorly maintained dependencies that change a lot in the outskirts of crates.io, but I never ran into noteworthy issues with libraries that are well maintained yet limited in scope (such as TL;DR Compared to many other sw ecosystems, dependency hell is much less pronounced when dealing with Rust. I say this to spark some hope with you guys, that maybe not all dependency usage is hurtful, especially when you oxidize. While I question the excessive extent of dependency avoidance to some degree, I do respect the strive for close to zero dependencies! I think its safe to assume that this PR will not reach consensus w/r/t the approach taken, so I'm closing it. Again, thank you for the discussion! |
This commit removes all unsafe (entirely related to transmuting) from the code. Instead, Google's zerocopy crate is used to do the transmuting. Being widely used and thoroughly vetted, it is less likely to cause hidden soundness issues.
The impact to the dependencies is minimal, i. e. the only extra crates added to the
Cargo.lock
arebyteorder
,zerocopy
andzerocopy-derive
.