sfackler / r2d2

A generic connection pool for Rust
Apache License 2.0
1.51k stars 82 forks source link

add #![forbid(unsafe_code)] #112

Closed rursprung closed 3 years ago

rursprung commented 4 years ago

this crate does not use unsafe code, so we can forbid it completely to ensure that it won't be added later on and to tell analysers that this is the case.

for more information see also here: https://github.com/rust-secure-code/safety-dance

sfackler commented 4 years ago

If I want to add unsafe code later on, I would just remove the annotation - what exactly does this ensure?

rursprung commented 4 years ago

sorry, i could've been more explicit on this: yes, of course you can always remove it later when you need to add unsafe code. but: this will have to be a very explicit decision with the first block of unsafe code - it can't just "sneak" in. it also helps with analysers (e.g. cargo-geiger, see link in the commit-msg), though these of course also scan for the unsafe blocks themselves. basically, as i see it, this is a safety-net for maintainers on one hand (ensuring that unsafe code isn't just added w/o proper deliberation) and consumers on the other hand (they don't need to dig too deep into the code of the libraries they use: they can be sure that the library is not doing anything unsafe (unless there's a bug in rustc or some ambiguity in the rust spec)).

sfackler commented 4 years ago

it also helps with analysers (e.g. cargo-geiger, see link in the commit-msg),

The existence of #![forbid(unsafe_code)] does not mean that a dependency does not contain unsafe code. Cargo passes --cap-lints=warn to rustc when building dependencies, which overrides the forbid.

rursprung commented 4 years ago

but you can only publish the dependency to crates.io if it builds and it must not have any unsafe code in order to build, so any crate you can get from crates.io should build. so using a crate from crates.io which has #![forbid(unsafe_code)] means that you're using a crate which doesn't have unsafe code. or am i missing something?

sfackler commented 4 years ago

but you can only publish the dependency to crates.io if it builds

No, that is not the case.