[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor/master] docs: More Rust coding standards w.r.t. fuzzing and safety.
commit aeef8a093f517bf8652a7ee9f46429f5e5b33d22
Author: Isis Lovecruft <isis@xxxxxxxxxxxxxx>
Date: Wed Aug 30 21:54:41 2017 +0000
docs: More Rust coding standards w.r.t. fuzzing and safety.
---
doc/HACKING/CodingStandardsRust.md | 100 +++++++++++++++++++++++++++++++++++--
1 file changed, 97 insertions(+), 3 deletions(-)
diff --git a/doc/HACKING/CodingStandardsRust.md b/doc/HACKING/CodingStandardsRust.md
index 41b76263a..fc4977ed9 100644
--- a/doc/HACKING/CodingStandardsRust.md
+++ b/doc/HACKING/CodingStandardsRust.md
@@ -140,6 +140,13 @@ Finally, to write your benchmark code, in
}
}
+ Fuzzing
+---------
+
+If you wish to fuzz parts of your code, please see the
+[`cargo fuzz`](https://github.com/rust-fuzz/cargo-fuzz) crate, which uses
+[libfuzzer-sys](https://github.com/rust-fuzz/libfuzzer-sys).
+
Safety
--------
@@ -241,9 +248,96 @@ Here are some additional bits of advice and rules:
6. Type safety
- Wherever possible and sensical, you SHOULD create new types, either as tuple
- structs (e.g. `struct MyInteger(pub u32)`) or as type aliases (e.g. `pub type
- MyInteger = u32`).
+ Wherever possible and sensical, you SHOULD create new types in a
+ manner which prevents type confusion or misuse. For example,
+ rather than using an untyped mapping between strings and integers
+ like so:
+
+ use std::collections::HashMap;
+
+ pub fn get_elements_with_over_9000_points(map: &HashMap<String, usize>) -> Vec<String> {
+ ...
+ }
+
+ It would be safer to define a new type, such that some other usage
+ of `HashMap<String, usize>` cannot be confused for this type:
+
+ pub struct DragonBallZPowers(pub HashMap<String, usize>);
+
+ impl DragonBallZPowers {
+ pub fn over_nine_thousand<'a>(&'a self) -> Vec<&'a String> {
+ let mut powerful_enough: Vec<&'a String> = Vec::with_capacity(5);
+
+ for (character, power) in &self.0 {
+ if *power > 9000 {
+ powerful_enough.push(character);
+ }
+ }
+ powerful_enough
+ }
+ }
+
+ Note the following code, which uses Rust's type aliasing, is valid
+ but it does NOT meet the desired type safety goals:
+
+ pub type Power = usize;
+
+ pub fn over_nine_thousand(power: &Power) -> bool {
+ if *power > 9000 {
+ return true;
+ }
+ false
+ }
+
+ // We can still do the following:
+ let his_power: usize = 9001;
+ over_nine_thousand(&his_power);
+
+7. Unsafe mucking around with lifetimes
+
+ Because lifetimes are technically, in type theory terms, a kind, i.e. a
+ family of types, individual lifetimes can be treated as types. For example,
+ one can arbitrarily extend and shorten lifetime using `std::mem::transmute`:
+
+ struct R<'a>(&'a i32);
+
+ unsafe fn extend_lifetime<'b>(r: R<'b>) -> R<'static> {
+ std::mem::transmute::<R<'b>, R<'static>>(r)
+ }
+
+ unsafe fn shorten_invariant_lifetime<'b, 'c>(r: &'b mut R<'static>) -> &'b mut R<'c> {
+ std::mem::transmute::<&'b mut R<'static>, &'b mut R<'c>>(r)
+ }
+
+ Calling `extend_lifetime()` would cause an `R` passed into it to live forever
+ for the life of the program (the `'static` lifetime). Similarly,
+ `shorten_invariant_lifetime()` could be used to take something meant to live
+ forever, and cause it to disappear! This is incredibly unsafe. If you're
+ going to be mucking around with lifetimes like this, first, you better have
+ an extremely good reason, and second, you may as be honest and explicit about
+ it, and for ferris' sake just use a raw pointer.
+
+ In short, just because lifetimes can be treated like types doesn't mean you
+ should do it.
+
+8. Doing excessively unsafe things when there's a safer alternative
+
+ Similarly to #7, often there are excessively unsafe ways to do a task and a
+ simpler, safer way. You MUST choose the safer option where possible.
+
+ For example, `std::mem::transmute` can be abused in ways where casting with
+ `as` would be both simpler and safer:
+
+ // Don't do this
+ let ptr = &0;
+ let ptr_num_transmute = unsafe { std::mem::transmute::<&i32, usize>(ptr)};
+
+ // Use an `as` cast instead
+ let ptr_num_cast = ptr as *const i32 as usize;
+
+ In fact, using `std::mem::transmute` for *any* reason is a code smell and as
+ such SHOULD be avoided.
+
Whitespace & Formatting
-------------------------
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits