Cryptographically verifiable, distributed dependency reviews
Add the last reviewed version to Cargo.toml / [dependencies]:
scopeguard = "1.1.0"
Please, use mobile in landscape.
Filter reviews clicking on the numbers in the summary.
Full column names in tooltip hints: rating Negative, rating Neutral, rating Positive, rating Strong, thoroughness, understanding, reviews count.
Looks okay but I'd prefer to just avoid needing it.
No malicious code. The unsafe
parts seems sane, but they could use a
a review by someone more confident with unsafe
.
© bestia.dev 2023, MIT License, Version: 2023.608.1636
Open source repository for this web app: https://github.com/bestia-dev/cargo_crev_web/
Small, stable, and highly useful.
Most of the code is straightforward but to increase usability there are three
unsafe
blocks. It seems feasible to write a completely safe version of theAPI without hurting the main use case too much. The benefit, however, would
be slim except for those focussed on purging even well-reviewed local
unsafe
. Let's explain how we agree with all reasoning, and documentation.src/lib.rs:351:
ScopeGuard::into_inner
: Here it performs a manual move froma to-be-forgotten value that can't be destructured due to the rules around
Drop types. It's slightly awkward to use
mem::forget
after the copyinstead of wrapping the value in a
ManuallyDrop
at the start but thecomment correctly explains why the order is equivalent and still sound. It
also seems odd that this pseudo-destructuring does not return the function
instance but only the value and thus always drops the functor which
potentially also drops any contained captured value and thus might panic.
That seems more ergonomic in the common case or when a pure function is used,
so it isn't critical.
src/lib.rs:422:
impl Sync
: This is likely the most involved here but thecomment explains it well enough. It might have been/be beneficial to create a
very small newtype wrapper that enforces that no such access by reference can
take place in future versions of the crate also. This would also reduce the
mental load, as this impl with all its type bounds would not be explicitly
required. It would nevertheless not reduce the amount of necessary soundness
reasoning so this is just polish.
src/lib.rs:455:
Drop::drop for ScopeGuard
: Same asinto_inner
but sincethis is within the Drop impl there is no need (and possibility) to forget
self
so this is self-explanatory. This could share a common core with theinto_inner
method, a private inherent method thatptr::read
's the storedvalues. Avoiding duplication here would be more important if different ways
of destructuring existed.