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

Forbid unsafe code in the core crate #56

Merged
merged 1 commit into from
Aug 9, 2019
Merged

Conversation

Shnatsel
Copy link
Contributor

@Shnatsel Shnatsel commented Aug 9, 2019

Now that the core crate is 100% safe code while still being faster than the C version, it seems like a good idea to add #![forbid(unsafe_code)] annotation to keep it that way.

This annotation makes the compiler reject any unsafe blocks in the crate as long as it's present. This discourages people from trying to add unsafe blocks back in unless they're absolutely necessary. This is also picked up as a signal by some of the tooling, e.g. cargo-geiger.

@oyvindln oyvindln merged commit 23a6759 into Frommi:master Aug 9, 2019
@oyvindln
Copy link
Collaborator

oyvindln commented Aug 9, 2019

👍

@matklad
Copy link
Contributor

matklad commented Aug 27, 2019

Now that the core crate is 100% safe code while still being faster than the C version

@oyvindln @Frommi you probably should blog about this :-)

@matklad
Copy link
Contributor

matklad commented Aug 27, 2019

Also, this seems like a good occasion to switch Cargo to miniz_oxide? IIRC it still uses zlib (https://github.com/rust-lang/cargo/blob/master/Cargo.toml#L32). Cc @ehuss

@Shnatsel
Copy link
Contributor Author

I don't think cargo would benefit from this conversion; after all, any code downloaded by Cargo is already trusted because you're about to compile it. In this case compatibility trumps safety.

However, I do believe it's time to make flate2 use miniz_oxide backend by default instead of miniz.

@matklad
Copy link
Contributor

matklad commented Aug 27, 2019

I agree that there’s little speed or security benefit in the switch. But having less C dependencies is an important operational benefit, as compiling C code might be tricky. It’s unlikely that cargo will ever be completely free from c deps: OpenSSL and libgit2 are hard to replace. But even just removing zlib from https://users.rust-lang.org/t/solved-how-do-i-build-cargo-on-nixos/7620 would be useful:)

Compatibility is of course super important, but should be easy to check: just uncompressed every crate file from crates.io.

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.

3 participants