Skip to content

Commit

Permalink
Resolve code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
kalaninja committed Nov 8, 2024
1 parent 2f6c267 commit 0a5b7e5
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 21 deletions.
10 changes: 7 additions & 3 deletions src/bit_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,13 @@ impl<O: BitOrder, T: BitStore + Decode> Decode for BitVec<T, O> {
fn skip<I: Input>(input: &mut I) -> Result<(), Error> {
let Compact(bits) = <Compact<u32>>::decode(input)?;
let len = bitvec::mem::elts::<T>(bits as usize);
match T::encoded_fixed_size() {
Some(size) => input.skip(size * len),
None => Result::from_iter((0..len).map(|_| T::skip(input))),

// Attempt to get the fixed size and check for overflow
if let Some(size) = T::encoded_fixed_size().and_then(|size| size.checked_mul(len)) {
input.skip(size)
} else {
// Fallback when there is no fixed size or on overflow
Result::from_iter((0..len).map(|_| T::skip(input)))
}
}
}
Expand Down
69 changes: 51 additions & 18 deletions src/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ pub trait Decode: Sized {
unsafe { Ok(DecodeFinished::assert_decoding_finished()) }
}

/// Attempt to skip the encoded value from input.
/// Attempt to skip the encoded value from input without validating it.
///
/// The default implementation of this function is skipping the fixed encoded size
/// if it is known. Otherwise, it is just calling [`Decode::decode`].
Expand Down Expand Up @@ -474,7 +474,7 @@ impl Input for BytesCursor {

fn skip(&mut self, len: usize) -> Result<(), Error> {
if len > self.bytes.len() - self.position {
return Err("Not enough data to skip".into())
return Err("Not enough data to skip".into());
}

self.position += len;
Expand Down Expand Up @@ -770,8 +770,8 @@ impl<T: Decode, E: Decode> Decode for Result<T, E> {
.read_byte()
.map_err(|e| e.chain("Could not result variant byte for `Result`"))?
{
0 => T::skip(input),
1 => E::skip(input),
0 => T::skip(input).map_err(|e| e.chain("Could not skip `Result::Ok(T)`")),
1 => E::skip(input).map_err(|e| e.chain("Could not skip `Result::Error(E)`")),
_ => Err("unexpected first byte decoding Result".into()),
}
}
Expand Down Expand Up @@ -863,10 +863,14 @@ impl<T: Decode> Decode for Option<T> {
.map_err(|e| e.chain("Could not decode variant byte for `Option`"))?
{
0 => Ok(()),
1 => T::skip(input),
1 => T::skip(input).map_err(|e| e.chain("Could not skip `Option::Some(T)`")),
_ => Err("unexpected first byte decoding Option".into()),
}
}

fn encoded_fixed_size() -> Option<usize> {
Some(T::encoded_fixed_size()? + 1)
}
}

impl<T: DecodeWithMemTracking> DecodeWithMemTracking for Option<T> {}
Expand Down Expand Up @@ -999,16 +1003,16 @@ impl<T: Decode, const N: usize> Decode for [T; N] {
) -> Result<DecodeFinished, Error> {
let is_primitive = match <T as Decode>::TYPE_INFO {
| TypeInfo::U8 | TypeInfo::I8 => true,
| TypeInfo::U16 |
TypeInfo::I16 |
TypeInfo::U32 |
TypeInfo::I32 |
TypeInfo::U64 |
TypeInfo::I64 |
TypeInfo::U128 |
TypeInfo::I128 |
TypeInfo::F32 |
TypeInfo::F64 => cfg!(target_endian = "little"),
| TypeInfo::U16
| TypeInfo::I16
| TypeInfo::U32
| TypeInfo::I32
| TypeInfo::U64
| TypeInfo::I64
| TypeInfo::U128
| TypeInfo::I128
| TypeInfo::F32
| TypeInfo::F64 => cfg!(target_endian = "little"),
TypeInfo::Unknown => false,
};

Expand Down Expand Up @@ -1322,9 +1326,14 @@ impl<T: Decode> Decode for Vec<T> {

fn skip<I: Input>(input: &mut I) -> Result<(), Error> {
let Compact(len) = <Compact<u32>>::decode(input)?;
match T::encoded_fixed_size() {
Some(size) => input.skip(size * len as usize),
None => Result::from_iter((0..len).map(|_| T::skip(input))),

// Attempt to get the fixed size and check for overflow
if let Some(size) = T::encoded_fixed_size().and_then(|size| size.checked_mul(len as usize))
{
input.skip(size)
} else {
// Fallback when there is no fixed size or on overflow
Result::from_iter((0..len).map(|_| T::skip(input)))
}
}
}
Expand Down Expand Up @@ -1376,6 +1385,10 @@ impl<K: Decode + Ord, V: Decode> Decode for BTreeMap<K, V> {
result
})
}

fn skip<I: Input>(input: &mut I) -> Result<(), Error> {
Vec::<(K, V)>::skip(input)
}
}

impl<K: DecodeWithMemTracking, V: DecodeWithMemTracking> DecodeWithMemTracking for BTreeMap<K, V> where
Expand All @@ -1398,6 +1411,10 @@ impl<T: Decode + Ord> Decode for BTreeSet<T> {
result
})
}

fn skip<I: Input>(input: &mut I) -> Result<(), Error> {
Vec::<T>::skip(input)
}
}
impl<T: DecodeWithMemTracking> DecodeWithMemTracking for BTreeSet<T> where BTreeSet<T>: Decode {}

Expand All @@ -1422,6 +1439,10 @@ impl<T: Decode> Decode for LinkedList<T> {
result
})
}

fn skip<I: Input>(input: &mut I) -> Result<(), Error> {
Vec::<T>::skip(input)
}
}

impl<T: DecodeWithMemTracking> DecodeWithMemTracking for LinkedList<T> where LinkedList<T>: Decode {}
Expand All @@ -1435,6 +1456,10 @@ impl<T: Decode + Ord> Decode for BinaryHeap<T> {
fn decode<I: Input>(input: &mut I) -> Result<Self, Error> {
Ok(Vec::decode(input)?.into())
}

fn skip<I: Input>(input: &mut I) -> Result<(), Error> {
Vec::<T>::skip(input)
}
}
impl<T: DecodeWithMemTracking> DecodeWithMemTracking for BinaryHeap<T> where BinaryHeap<T>: Decode {}

Expand Down Expand Up @@ -1797,6 +1822,10 @@ where
Ok(Range { start, end })
}

fn skip<I: Input>(input: &mut I) -> Result<(), Error> {
<(T, T)>::skip(input)
}

fn encoded_fixed_size() -> Option<usize> {
<(T, T)>::encoded_fixed_size()
}
Expand Down Expand Up @@ -1827,6 +1856,10 @@ where
Ok(RangeInclusive::new(start, end))
}

fn skip<I: Input>(input: &mut I) -> Result<(), Error> {
<(T, T)>::skip(input)
}

fn encoded_fixed_size() -> Option<usize> {
<(T, T)>::encoded_fixed_size()
}
Expand Down

0 comments on commit 0a5b7e5

Please sign in to comment.