Frommi / miniz_oxide

Rust replacement for miniz
MIT License
174 stars 49 forks source link

Forbid unsafe code in the core crate #56

Closed Shnatsel closed 5 years ago

Shnatsel commented 5 years ago

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 commented 5 years ago

:+1:

matklad commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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.