*: avoid Result::is_{ok,err}

These methods discard information. Discarding information is bad.
reviewable/pr1194/r1
Tamir Duberstein 2 weeks ago
parent a25e355ba7
commit 2d782606fe

@ -11,5 +11,8 @@ edition.workspace = true
[dependencies] [dependencies]
aya-log-common = { path = "../aya-log-common", version = "^0.1.14", default-features = false } aya-log-common = { path = "../aya-log-common", version = "^0.1.14", default-features = false }
[dev-dependencies]
assert_matches = { workspace = true }
[lib] [lib]
path = "src/lib.rs" path = "src/lib.rs"

@ -128,11 +128,10 @@ pub fn parse(format_string: &str) -> Result<Vec<Fragment>, String> {
#[cfg(test)] #[cfg(test)]
mod test { mod test {
use assert_matches::assert_matches;
use super::*; use super::*;
// TODO(https://github.com/rust-lang/rust-clippy/issues/13885): narrow this to just the specific
// strings when that doesn't trip the lint.
#[allow(clippy::literal_string_with_formatting_args)]
#[test] #[test]
fn test_parse() { fn test_parse() {
assert_eq!( assert_eq!(
@ -160,9 +159,9 @@ mod test {
}), }),
]) ])
); );
assert!(parse("foo {:}").is_err()); assert_matches!(parse("foo {:}"), Err(_));
assert!(parse("foo { bar").is_err()); assert_matches!(parse("foo { bar"), Err(_));
assert!(parse("foo } bar").is_err()); assert_matches!(parse("foo } bar"), Err(_));
assert!(parse("foo { bar }").is_err()); assert_matches!(parse("foo { bar }"), Err(_));
} }
} }

@ -20,15 +20,17 @@ use crate::{
/// # Examples /// # Examples
/// ///
/// ```no_run /// ```no_run
/// # use assert_matches::assert_matches;
/// # let mut bpf = aya::Ebpf::load(&[])?; /// # let mut bpf = aya::Ebpf::load(&[])?;
/// use aya::maps::MapError;
/// use aya::maps::bloom_filter::BloomFilter; /// use aya::maps::bloom_filter::BloomFilter;
/// ///
/// let mut bloom_filter = BloomFilter::try_from(bpf.map_mut("BLOOM_FILTER").unwrap())?; /// let mut bloom_filter = BloomFilter::try_from(bpf.map_mut("BLOOM_FILTER").unwrap())?;
/// ///
/// bloom_filter.insert(1, 0)?; /// bloom_filter.insert(1, 0)?;
/// ///
/// assert!(bloom_filter.contains(&1, 0).is_ok()); /// assert_matches!(bloom_filter.contains(&1, 0), Ok(()));
/// assert!(bloom_filter.contains(&2, 0).is_err()); /// assert_matches!(bloom_filter.contains(&2, 0), Err(MapError::ElementNotFound));
/// ///
/// # Ok::<(), aya::EbpfError>(()) /// # Ok::<(), aya::EbpfError>(())
/// ``` /// ```
@ -131,7 +133,7 @@ mod tests {
fn test_new_ok() { fn test_new_ok() {
let map = new_map(new_obj_map()); let map = new_map(new_obj_map());
assert!(BloomFilter::<_, u32>::new(&map).is_ok()); let _: BloomFilter<_, u32> = BloomFilter::new(&map).unwrap();
} }
#[test] #[test]
@ -139,7 +141,7 @@ mod tests {
let map = new_map(new_obj_map()); let map = new_map(new_obj_map());
let map = Map::BloomFilter(map); let map = Map::BloomFilter(map);
assert!(BloomFilter::<_, u32>::try_from(&map).is_ok()) let _: BloomFilter<_, u32> = map.try_into().unwrap();
} }
#[test] #[test]
@ -168,7 +170,7 @@ mod tests {
_ => sys_error(EFAULT), _ => sys_error(EFAULT),
}); });
assert!(bloom_filter.insert(0, 42).is_ok()); assert_matches!(bloom_filter.insert(0, 42), Ok(()));
} }
#[test] #[test]

@ -179,23 +179,23 @@ mod tests {
#[test] #[test]
fn test_new_ok() { fn test_new_ok() {
let map = new_map(new_obj_map()); let map = new_map(new_obj_map());
assert!(HashMap::<_, u32, u32>::new(&map).is_ok()); let _: HashMap<_, u32, u32> = HashMap::new(&map).unwrap();
} }
#[test] #[test]
fn test_try_from_ok() { fn test_try_from_ok() {
let map = new_map(new_obj_map()); let map = new_map(new_obj_map());
let map = Map::HashMap(map); let map = Map::HashMap(map);
assert!(HashMap::<_, u32, u32>::try_from(&map).is_ok()) let _: HashMap<_, u32, u32> = map.try_into().unwrap();
} }
#[test] #[test]
fn test_try_from_ok_lru() { fn test_try_from_ok_lru() {
let map_data = || new_map(test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_LRU_HASH)); let map_data = || new_map(test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_LRU_HASH));
let map = Map::HashMap(map_data()); let map = Map::HashMap(map_data());
assert!(HashMap::<_, u32, u32>::try_from(&map).is_ok()); let _: HashMap<_, u32, u32> = map.try_into().unwrap();
let map = Map::LruHashMap(map_data()); let map = Map::LruHashMap(map_data());
assert!(HashMap::<_, u32, u32>::try_from(&map).is_ok()) let _: HashMap<_, u32, u32> = map.try_into().unwrap();
} }
#[test] #[test]
@ -224,7 +224,7 @@ mod tests {
_ => sys_error(EFAULT), _ => sys_error(EFAULT),
}); });
assert!(hm.insert(1, 42, 0).is_ok()); assert_matches!(hm.insert(1, 42, 0), Ok(()));
} }
#[test] #[test]
@ -240,7 +240,7 @@ mod tests {
_ => sys_error(EFAULT), _ => sys_error(EFAULT),
}); });
assert!(hm.insert(Box::new(1), Box::new(42), 0).is_ok()); assert_matches!(hm.insert(Box::new(1), Box::new(42), 0), Ok(()));
} }
#[test] #[test]
@ -269,7 +269,7 @@ mod tests {
_ => sys_error(EFAULT), _ => sys_error(EFAULT),
}); });
assert!(hm.remove(&1).is_ok()); assert_matches!(hm.remove(&1), Ok(()));
} }
#[test] #[test]

@ -174,16 +174,16 @@ mod tests {
let map = Map::PerCpuHashMap(test_utils::new_map(test_utils::new_obj_map::<u32>( let map = Map::PerCpuHashMap(test_utils::new_map(test_utils::new_obj_map::<u32>(
BPF_MAP_TYPE_PERCPU_HASH, BPF_MAP_TYPE_PERCPU_HASH,
))); )));
assert!(PerCpuHashMap::<_, u32, u32>::try_from(&map).is_ok()) let _: PerCpuHashMap<_, u32, u32> = map.try_into().unwrap();
} }
#[test] #[test]
fn test_try_from_ok_lru() { fn test_try_from_ok_lru() {
let map_data = let map_data =
|| test_utils::new_map(test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_LRU_PERCPU_HASH)); || test_utils::new_map(test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_LRU_PERCPU_HASH));
let map = Map::PerCpuHashMap(map_data()); let map = Map::PerCpuHashMap(map_data());
assert!(PerCpuHashMap::<_, u32, u32>::try_from(&map).is_ok()); let _: PerCpuHashMap<_, u32, u32> = map.try_into().unwrap();
let map = Map::PerCpuLruHashMap(map_data()); let map = Map::PerCpuLruHashMap(map_data());
assert!(PerCpuHashMap::<_, u32, u32>::try_from(&map).is_ok()) let _: PerCpuHashMap<_, u32, u32> = map.try_into().unwrap();
} }
#[test] #[test]
fn test_get_not_found() { fn test_get_not_found() {

@ -260,7 +260,7 @@ mod tests {
fn test_new_ok() { fn test_new_ok() {
let map = new_map(new_obj_map()); let map = new_map(new_obj_map());
assert!(LpmTrie::<_, u32, u32>::new(&map).is_ok()); let _: LpmTrie<_, u32, u32> = LpmTrie::new(&map).unwrap();
} }
#[test] #[test]
@ -268,7 +268,7 @@ mod tests {
let map = new_map(new_obj_map()); let map = new_map(new_obj_map());
let map = Map::LpmTrie(map); let map = Map::LpmTrie(map);
assert!(LpmTrie::<_, u32, u32>::try_from(&map).is_ok()) let _: LpmTrie<_, u32, u32> = map.try_into().unwrap();
} }
#[test] #[test]
@ -301,7 +301,7 @@ mod tests {
_ => sys_error(EFAULT), _ => sys_error(EFAULT),
}); });
assert!(trie.insert(&key, 1, 0).is_ok()); assert_matches!(trie.insert(&key, 1, 0), Ok(()));
} }
#[test] #[test]
@ -334,7 +334,7 @@ mod tests {
_ => sys_error(EFAULT), _ => sys_error(EFAULT),
}); });
assert!(trie.remove(&key).is_ok()); assert_matches!(trie.remove(&key), Ok(()));
} }
#[test] #[test]

@ -634,11 +634,11 @@ mod tests {
assert_eq!(*l1_detached.borrow(), 0); assert_eq!(*l1_detached.borrow(), 0);
assert_eq!(*l2_detached.borrow(), 0); assert_eq!(*l2_detached.borrow(), 0);
assert!(links.remove(id1).is_ok()); links.remove(id1).unwrap();
assert_eq!(*l1_detached.borrow(), 1); assert_eq!(*l1_detached.borrow(), 1);
assert_eq!(*l2_detached.borrow(), 0); assert_eq!(*l2_detached.borrow(), 0);
assert!(links.remove(id2).is_ok()); links.remove(id2).unwrap();
assert_eq!(*l1_detached.borrow(), 1); assert_eq!(*l1_detached.borrow(), 1);
assert_eq!(*l2_detached.borrow(), 1); assert_eq!(*l2_detached.borrow(), 1);
} }
@ -678,7 +678,7 @@ mod tests {
let id1 = links.insert(l1).unwrap(); let id1 = links.insert(l1).unwrap();
links.insert(l2).unwrap(); links.insert(l2).unwrap();
// manually remove one link // manually remove one link
assert!(links.remove(id1).is_ok()); links.remove(id1).unwrap();
assert_eq!(*l1_detached.borrow(), 1); assert_eq!(*l1_detached.borrow(), 1);
assert_eq!(*l2_detached.borrow(), 0); assert_eq!(*l2_detached.borrow(), 0);
} }
@ -710,7 +710,7 @@ mod tests {
assert_eq!(*l2_detached.borrow(), 1); assert_eq!(*l2_detached.borrow(), 1);
// manually detach l1 // manually detach l1
assert!(owned_l1.detach().is_ok()); owned_l1.detach().unwrap();
assert_eq!(*l1_detached.borrow(), 1); assert_eq!(*l1_detached.borrow(), 1);
assert_eq!(*l2_detached.borrow(), 1); assert_eq!(*l2_detached.borrow(), 1);
} }

@ -716,7 +716,7 @@ mod tests {
let align_bytes = aligned_slice(&mut debug_bytes); let align_bytes = aligned_slice(&mut debug_bytes);
let debug_obj = object::File::parse(&*align_bytes).expect("got debug obj"); let debug_obj = object::File::parse(&*align_bytes).expect("got debug obj");
assert!(verify_build_ids(&main_obj, &debug_obj, "symbol_name").is_ok()); verify_build_ids(&main_obj, &debug_obj, "symbol_name").unwrap();
} }
#[test] #[test]

@ -441,9 +441,9 @@ mod tests {
parse_cpu_ranges("0-5,6,7").unwrap(), parse_cpu_ranges("0-5,6,7").unwrap(),
(0..=7).collect::<Vec<_>>() (0..=7).collect::<Vec<_>>()
); );
assert!(parse_cpu_ranges("").is_err()); assert_matches!(parse_cpu_ranges(""), Err(_));
assert!(parse_cpu_ranges("0-1,2-").is_err()); assert_matches!(parse_cpu_ranges("0-1,2-"), Err(_));
assert!(parse_cpu_ranges("foo").is_err()); assert_matches!(parse_cpu_ranges("foo"), Err(_));
} }
#[test] #[test]

Loading…
Cancel
Save