Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 28 additions & 50 deletions der/src/asn1/set_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,8 @@ where
/// Insert an item into this [`SetOf`].
///
/// # Errors
/// If there's a duplicate or sorting error.
/// If there's a sorting error.
pub fn insert(&mut self, item: T) -> Result<(), Error> {
check_duplicate(&item, self.iter())?;
self.try_push(item)?;
der_sort(self.inner.as_mut())
}
Expand Down Expand Up @@ -304,9 +303,8 @@ where
/// Insert an item into this [`SetOfVec`]. Must be unique.
///
/// # Errors
/// If `item` is a duplicate or a sorting error occurred.
/// If a sorting error occurred.
pub fn insert(&mut self, item: T) -> Result<(), Error> {
check_duplicate(&item, self.iter())?;
self.inner.push(item);
der_sort(&mut self.inner)
}
Expand Down Expand Up @@ -488,29 +486,10 @@ where
}
}

/// Check if the given item is a duplicate, given an iterator over sorted items (which we can
/// short-circuit once we hit `Ordering::Less`.
fn check_duplicate<'a, T, I>(item: &T, iter: I) -> Result<(), Error>
where
T: DerOrd + 'a,
I: Iterator<Item = &'a T>,
{
for item2 in iter {
match item.der_cmp(item2)? {
Ordering::Less => return Ok(()), // all remaining items are greater
Ordering::Equal => return Err(ErrorKind::SetDuplicate.into()),
Ordering::Greater => continue,
}
}

Ok(())
}

/// Ensure set elements are lexicographically ordered using [`DerOrd`].
fn check_der_ordering<T: DerOrd>(a: &T, b: &T) -> Result<(), Error> {
match a.der_cmp(b)? {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably just change to:

Suggested change
match a.der_cmp(b)? {
if a.der_cmp(b)? == Ordering::Greater {

Ordering::Less => Ok(()),
Ordering::Equal => Err(ErrorKind::SetDuplicate.into()),
Ordering::Less | Ordering::Equal => Ok(()),
Ordering::Greater => Err(ErrorKind::SetOrdering.into()),
}
}
Expand All @@ -531,8 +510,7 @@ fn der_sort<T: DerOrd>(slice: &mut [T]) -> Result<(), Error> {

while j > 0 {
match slice[j - 1].der_cmp(&slice[j])? {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise this can probably switch to an if statement

Ordering::Less => break,
Ordering::Equal => return Err(ErrorKind::SetDuplicate.into()),
Ordering::Less | Ordering::Equal => break,
Ordering::Greater => {
slice.swap(j - 1, j);
j -= 1;
Expand All @@ -549,7 +527,6 @@ fn der_sort<T: DerOrd>(slice: &mut [T]) -> Result<(), Error> {
mod tests {
#[cfg(feature = "alloc")]
use super::SetOfVec;
use crate::ErrorKind;
#[cfg(feature = "heapless")]
use {super::SetOf, crate::DerOrd};

Expand All @@ -560,13 +537,22 @@ mod tests {
setof.insert(42).unwrap();
assert_eq!(setof.len(), 1);
assert_eq!(*setof.iter().next().unwrap(), 42);
}

// Ensure duplicates are disallowed
assert_eq!(
setof.insert(42).unwrap_err().kind(),
ErrorKind::SetDuplicate
);
#[cfg(feature = "heapless")]
#[test]
fn setof_insert_duplicate() {
let mut setof = SetOf::<u8, 10>::new();
setof.insert(42).unwrap();
assert_eq!(setof.len(), 1);

setof.insert(42).unwrap();

let mut iter = setof.iter();

assert_eq!(setof.len(), 2);
assert_eq!(*iter.next().unwrap(), 42);
assert_eq!(*iter.next().unwrap(), 42);
}

#[cfg(feature = "heapless")]
Expand All @@ -577,14 +563,6 @@ mod tests {
assert!(set.iter().copied().eq([0, 1, 2, 3, 65535]));
}

#[cfg(feature = "heapless")]
#[test]
fn setof_tryfrom_array_reject_duplicates() {
let arr = [1u16, 1];
let err = SetOf::try_from(arr).err().unwrap();
assert_eq!(err.kind(), ErrorKind::SetDuplicate);
}

#[cfg(feature = "heapless")]
#[test]
fn setof_valueord_value_cmp() {
Expand All @@ -603,14 +581,14 @@ mod tests {
let mut setof = SetOfVec::new();
setof.insert(42).unwrap();
assert_eq!(setof.len(), 1);
assert_eq!(*setof.iter().next().unwrap(), 42);

// Ensure duplicates are disallowed
assert_eq!(
setof.insert(42).unwrap_err().kind(),
ErrorKind::SetDuplicate
);
assert_eq!(setof.len(), 1);
setof.insert(46).unwrap();

let mut iter = setof.iter();

assert_eq!(setof.len(), 2);
assert_eq!(*iter.next().unwrap(), 42);
assert_eq!(*iter.next().unwrap(), 46);
}

#[cfg(feature = "alloc")]
Expand All @@ -631,9 +609,9 @@ mod tests {

#[cfg(feature = "alloc")]
#[test]
fn setofvec_tryfrom_vec_reject_duplicates() {
fn setofvec_tryfrom_vec_allow_duplicates() {
let vec = vec![1u16, 1];
let err = SetOfVec::try_from(vec).err().unwrap();
assert_eq!(err.kind(), ErrorKind::SetDuplicate);
let set = SetOfVec::try_from(vec).unwrap();
assert_eq!(set.as_ref(), &[1, 1]);
}
}