Skip to content

Commit

Permalink
Reduce amount of unsafe code
Browse files Browse the repository at this point in the history
1. Compiler is actually smart enough to figure out that unsigned
   number + 1 is non-zero and optimises NonZeroU128::new(n + 1)
   correctly.  Get rid of the unsafe block and new_unchecked call.

2. Use ascii crate’s AsciiChar when formatting the bits slice.  This
   removes the need from converting slice of bytes into a str slice.
   Instead, we get AsciiStr for free which can be converted to str.

3. Remove unused as_chunks_mut method from the stdx crate.
  • Loading branch information
mina86 committed Nov 3, 2023
1 parent 9534c74 commit d838c6c
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 57 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ codegen-units = 1

[workspace.dependencies]
anchor-lang = {version = "0.28.0", features = ["init-if-needed"]}
ascii = "1.1.0"
base64 = { version = "0.21", default-features = false, features = ["alloc"] }
borsh = { version = "0.10.3", default-features = false }
derive_more = "0.99.17"
Expand Down
5 changes: 1 addition & 4 deletions common/blockchain/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,7 @@ impl<PK: crate::PubKey> ChainManager<PK> {
return None;
}
crate::Epoch::new_with(self.candidates.maybe_get_head()?, |total| {
// SAFETY: 1. ‘total / 2 ≥ 0’ thus ‘total / 2 + 1 > 0’.
// 2. ‘total / 2 <= u128::MAX / 2’ thus ‘total / 2 + 1 < u128::MAX’.
let quorum =
unsafe { NonZeroU128::new_unchecked(total.get() / 2 + 1) };
let quorum = NonZeroU128::new(total.get() / 2 + 1).unwrap();
// min_quorum_stake may be greater than total_stake so we’re not
// using .clamp to make sure we never return value higher than
// total_stake.
Expand Down
1 change: 1 addition & 0 deletions common/sealable-trie/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ version = "0.0.0"
edition = "2021"

[dependencies]
ascii.workspace = true
base64.workspace = true
borsh = { workspace = true, optional = true }
derive_more.workspace = true
Expand Down
58 changes: 25 additions & 33 deletions common/sealable-trie/src/bits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,11 +403,14 @@ impl<'a> Slice<'a> {

/// Returns bytes underlying the bit slice.
fn bytes(&self) -> &'a [u8] {
// We need to special-case zero length to make sure that in situation of
// non-zero offset and zero length we return an empty slice.
let len = match self.length {
0 => 0,
_ => (self.underlying_bits_length() + 7) / 8,
};
// SAFETY: `ptr` is guaranteed to be valid pointer point at offset+length valid bits.
// SAFETY: `ptr` is guaranteed to be valid pointer point at `offset +
// length` valid bits.
unsafe { core::slice::from_raw_parts(self.ptr, len) }
}

Expand Down Expand Up @@ -700,46 +703,35 @@ impl core::cmp::PartialEq<Owned> for Slice<'_> {

impl fmt::Display for Slice<'_> {
fn fmt(&self, fmtr: &mut fmt::Formatter<'_>) -> fmt::Result {
fn fmt(buf: &mut [u8], mut byte: u8) {
use ascii::AsciiChar;

fn fmt(buf: &mut [AsciiChar], mut byte: u8) {
for ch in buf.iter_mut().rev() {
*ch = b'0' + (byte & 1);
*ch = if byte & 1 == 1 { AsciiChar::_1 } else { AsciiChar::_0 };
byte >>= 1;
}
}

let (first, mid) = match self.bytes().split_first() {
None => return fmtr.write_str("∅"),
Some(pair) => pair,
};

let off = usize::from(self.offset);
let len = usize::from(self.length);
let mut buf = [0; 10];
fmt(&mut buf[2..], *first);
buf[0] = b'0';
buf[1] = b'b';
buf[2..2 + off].fill(b'.');

let (last, mid) = match mid.split_last() {
None => {
buf[2 + off + len..].fill(b'.');
let val = unsafe { core::str::from_utf8_unchecked(&buf) };
return fmtr.write_str(val);
let mut off = usize::from(self.offset);
let mut len = off + usize::from(self.length);
let mut buf = [AsciiChar::Null; 9];
buf[0] = AsciiChar::b;

fmtr.write_str(if self.length == 0 { "∅" } else { "0" })?;
for byte in self.bytes() {
fmt(&mut buf[1..], *byte);
buf[1..1 + off].fill(AsciiChar::Dot);
if len < 8 {
buf[1 + len..].fill(AsciiChar::Dot);
} else {
off = 0;
len -= 8 - off;
}
Some(pair) => pair,
};

fmtr.write_str(unsafe { core::str::from_utf8_unchecked(&buf) })?;
for byte in mid {
write!(fmtr, "_{:08b}", byte)?;
}
fmt(&mut buf[..9], *last);
buf[0] = b'_';
let len = (off + len) % 8;
if len != 0 {
buf[1 + len..].fill(b'.');
fmtr.write_str(<&ascii::AsciiStr>::from(&buf[..]).as_str())?;
buf[0] = AsciiChar::UnderScore;
}
fmtr.write_str(unsafe { core::str::from_utf8_unchecked(&buf[..9]) })
Ok(())
}
}

Expand Down
20 changes: 0 additions & 20 deletions common/stdx/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,28 +47,8 @@ pub fn rsplit_at<const R: usize>(xs: &[u8]) -> Option<(&[u8], &[u8; R])> {
Some((head, tail.try_into().unwrap()))
}

/// Splits the slice into a slice of `N`-element arrays and a remainder slice
/// with length strictly less than `N`.
///
/// This is simplified copy of Rust unstable `[T]::as_chunks_mut` method.
pub fn as_chunks_mut<const N: usize, T>(slice: &mut [T]) -> &mut [[T; N]] {
let () = AssertNonZero::<N>::OK;

let ptr = slice.as_mut_ptr().cast();
let len = slice.len() / N;
// SAFETY: We already panicked for zero, and ensured by construction
// that the length of the subslice is a multiple of N.
unsafe { core::slice::from_raw_parts_mut(ptr, len) }
}

/// Asserts, at compile time, that `A + B == S`.
struct AssertEqSum<const A: usize, const B: usize, const S: usize>;
impl<const A: usize, const B: usize, const S: usize> AssertEqSum<A, B, S> {
const OK: () = assert!(S == A + B);
}

/// Asserts, at compile time, that `N` is non-zero.
struct AssertNonZero<const N: usize>;
impl<const N: usize> AssertNonZero<N> {
const OK: () = assert!(N != 0);
}

0 comments on commit d838c6c

Please sign in to comment.