cleanup + switching to entry instead of find_key()

btrfs/src/lib.rs

@@ -36,7 +36,7 @@ pub mod std_io {
     impl<T: Read + Seek> VolumeIo for T {
         fn read(&mut self, dst: &mut [u8], address: u64) -> Result<(), Error> {
             self.seek(std::io::SeekFrom::Start(address))
-                .map_err(|a| Error::ReadFailed)?;
+                .map_err(|_| Error::ReadFailed)?;
             self.read_exact(dst).map_err(|_| Error::ReadFailed)
         }
 

btrfs/src/v2/mod.rs

@@ -1,11 +1,3 @@
-use core::{
-    cmp::Ordering,
-    fmt::Debug,
-    ops::{Bound, RangeBounds},
-};
-
-use crate::Error;
-
 pub mod error {
     use thiserror::Error;
 
@@ -63,7 +55,8 @@ pub trait Read {
 impl Read for std::fs::File {
     fn read(&self, dst: &mut [u8], address: u64) -> error::Result<()> {
         use std::os::unix::prelude::FileExt;
-        self.read_at(dst, address).map_err(|_| Error::ReadFailed)?;
+        self.read_at(dst, address)
+            .map_err(|_| error::Error::ReadFailed)?;
         Ok(())
     }
 }
@@ -71,275 +64,3 @@ impl Read for std::fs::File {
 pub mod file;
 pub mod tree;
 pub mod volume;
-
-pub fn cmp_start_bound<T: PartialOrd>(
-    rhs: &core::ops::Bound<T>,
-    lhs: &core::ops::Bound<T>,
-) -> Option<core::cmp::Ordering> {
-    match rhs {
-        core::ops::Bound::Included(r) => match lhs {
-            core::ops::Bound::Included(l) => r.partial_cmp(&l),
-            core::ops::Bound::Excluded(l) => match r.partial_cmp(&l) {
-                Some(core::cmp::Ordering::Equal) => Some(core::cmp::Ordering::Less),
-                i => i,
-            },
-            core::ops::Bound::Unbounded => Some(core::cmp::Ordering::Greater),
-        },
-        core::ops::Bound::Excluded(r) => match lhs {
-            core::ops::Bound::Excluded(l) => r.partial_cmp(&l),
-            core::ops::Bound::Included(l) => match r.partial_cmp(&l) {
-                Some(core::cmp::Ordering::Equal) => Some(core::cmp::Ordering::Greater),
-                i => i,
-            },
-            core::ops::Bound::Unbounded => Some(core::cmp::Ordering::Greater),
-        },
-        core::ops::Bound::Unbounded => match lhs {
-            core::ops::Bound::Unbounded => Some(core::cmp::Ordering::Equal),
-            _ => Some(core::cmp::Ordering::Less),
-        },
-    }
-}
-
-pub fn cmp_end_bound<T: PartialOrd>(
-    rhs: &core::ops::Bound<T>,
-    lhs: &core::ops::Bound<T>,
-) -> Option<core::cmp::Ordering> {
-    match rhs {
-        core::ops::Bound::Included(r) => match lhs {
-            core::ops::Bound::Included(l) => r.partial_cmp(&l),
-            core::ops::Bound::Excluded(l) => match r.partial_cmp(&l) {
-                Some(core::cmp::Ordering::Equal) => Some(core::cmp::Ordering::Greater),
-                i => i,
-            },
-            core::ops::Bound::Unbounded => Some(core::cmp::Ordering::Less),
-        },
-        core::ops::Bound::Excluded(r) => match lhs {
-            core::ops::Bound::Excluded(l) => r.partial_cmp(&l),
-            core::ops::Bound::Included(l) => match r.partial_cmp(&l) {
-                Some(core::cmp::Ordering::Equal) => Some(core::cmp::Ordering::Greater),
-                i => i,
-            },
-            core::ops::Bound::Unbounded => Some(core::cmp::Ordering::Less),
-        },
-        core::ops::Bound::Unbounded => match lhs {
-            core::ops::Bound::Unbounded => Some(core::cmp::Ordering::Equal),
-            _ => Some(core::cmp::Ordering::Greater),
-        },
-    }
-}
-
-#[derive(Debug, PartialEq, Eq)]
-struct StartBound<T: PartialOrd>(Bound<T>);
-impl<T: PartialOrd> PartialOrd for StartBound<T> {
-    fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> {
-        cmp_start_bound(&self.0, &other.0)
-    }
-}
-impl<T: Ord> Ord for StartBound<T> {
-    fn cmp(&self, other: &Self) -> core::cmp::Ordering {
-        self.partial_cmp(other).unwrap()
-    }
-}
-
-#[derive(Debug, PartialEq, Eq)]
-struct EndBound<T: PartialOrd>(Bound<T>);
-
-impl<T: PartialOrd> PartialOrd for EndBound<T> {
-    fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> {
-        cmp_end_bound(&self.0, &other.0)
-    }
-}
-impl<T: Ord> Ord for EndBound<T> {
-    fn cmp(&self, other: &Self) -> core::cmp::Ordering {
-        // safety: partial_cmp only returns None when returning T::partial_cmp
-        self.partial_cmp(other).unwrap()
-    }
-}
-
-pub fn range_countains_bound<T: Ord + Debug, R: RangeBounds<T>>(
-    range: R,
-    bound: Bound<&T>,
-) -> bool {
-    let start = &StartBound(bound);
-    let end = &EndBound(bound);
-    let r_start = &StartBound(range.start_bound());
-    let r_end = &EndBound(range.end_bound());
-    log::info!(
-        "start: {start:?} <=> {r_start:?}: {:?} {:?}",
-        start.cmp(r_start),
-        r_start.cmp(start)
-    );
-    log::info!(
-        "end: {end:?} <=> {r_end:?}: {:?} {:?}",
-        end.cmp(r_end),
-        r_end.cmp(end)
-    );
-    (start.cmp(r_start).is_ge() || r_start.cmp(start).is_le())
-        && (end.cmp(r_end).is_ge() || r_end.cmp(end).is_le())
-}
-
-#[cfg(test)]
-mod bound_ord_tests {
-    use core::{cmp::Ordering, ops::RangeBounds};
-
-    use super::*;
-    use test_log::test;
-
-    #[test]
-    fn start_bound_ord() {
-        assert_eq!(
-            cmp_start_bound(&Bound::Unbounded, &Bound::Included(0)),
-            Some(Ordering::Less)
-        );
-        assert_eq!(
-            cmp_start_bound::<i32>(&Bound::Unbounded, &Bound::Unbounded),
-            Some(Ordering::Equal)
-        );
-        assert_eq!(
-            cmp_start_bound(&Bound::Included(0), &Bound::Included(0)),
-            Some(Ordering::Equal)
-        );
-        assert_eq!(
-            cmp_start_bound(&Bound::Excluded(0), &Bound::Included(0)),
-            Some(Ordering::Greater)
-        );
-        assert_ne!(
-            cmp_start_bound(&Bound::Excluded(0), &Bound::Included(1)),
-            Some(Ordering::Greater)
-        );
-        // This is actually WRONG and is why we have to test both ways because I
-        // can't think of a way to actually determine how the 2 bounds are
-        // ordered without knowing the smallest discrete step size of T.
-        //
-        // In this case technically they should be equal, but for floats (which
-        // arent ord anyways?) this would be wrong
-        assert_eq!(
-            cmp_start_bound(&Bound::Included(1), &Bound::Excluded(0)),
-            Some(Ordering::Greater)
-        );
-
-        assert_eq!(
-            cmp_start_bound(&Bound::Included(0), &Bound::Excluded(1)),
-            Some(Ordering::Less)
-        );
-    }
-
-    #[test]
-    fn end_bound_ord() {
-        assert_eq!(
-            cmp_end_bound::<i32>(&Bound::Unbounded, &Bound::Unbounded),
-            Some(Ordering::Equal)
-        );
-        assert_eq!(
-            cmp_end_bound(&Bound::Unbounded, &Bound::Included(0)),
-            Some(Ordering::Greater)
-        );
-        assert_eq!(
-            cmp_end_bound(&Bound::Included(0), &Bound::Included(0)),
-            Some(Ordering::Equal)
-        );
-        assert_eq!(
-            cmp_end_bound(&Bound::Excluded(0), &Bound::Included(0)),
-            Some(Ordering::Greater)
-        );
-        assert_ne!(
-            cmp_end_bound(&Bound::Excluded(0), &Bound::Included(1)),
-            Some(Ordering::Greater)
-        );
-        // This is actually WRONG and is why we have to test both ways because I
-        // can't think of a way to actually determine how the 2 bounds are
-        // ordered without knowing the smallest discrete step size of T.
-        //
-        // In this case technically they should be equal, but for floats (which
-        // arent ord anyways?) this would be wrong
-        assert_eq!(
-            cmp_end_bound(&Bound::Included(1), &Bound::Excluded(0)),
-            Some(Ordering::Greater)
-        );
-
-        assert_eq!(
-            cmp_end_bound(&Bound::Included(0), &Bound::Excluded(1)),
-            Some(Ordering::Less)
-        );
-    }
-
-    #[test]
-    fn test_bound_ord() {
-        let r1 = 0..4;
-        let r2 = 2..3;
-
-        assert_eq!(
-            cmp_start_bound(&r1.start_bound(), &r2.start_bound()),
-            Some(core::cmp::Ordering::Less)
-        );
-        assert_eq!(
-            cmp_end_bound(&r1.end_bound(), &r2.end_bound()),
-            Some(core::cmp::Ordering::Greater)
-        );
-
-        assert_eq!(
-            cmp_start_bound(&r2.start_bound(), &r1.start_bound()),
-            Some(core::cmp::Ordering::Greater)
-        );
-        assert_eq!(
-            cmp_end_bound(&r2.end_bound(), &r1.end_bound()),
-            Some(core::cmp::Ordering::Less)
-        );
-
-        let r1 = 0..=8;
-        let r2 = 0..9;
-
-        assert_eq!(
-            cmp_start_bound(&r1.start_bound(), &r2.start_bound()),
-            Some(core::cmp::Ordering::Equal)
-        );
-        assert_eq!(
-            cmp_end_bound(&r1.end_bound(), &r2.end_bound()),
-            Some(core::cmp::Ordering::Less)
-        );
-        assert_eq!(
-            cmp_end_bound(&r2.end_bound(), &r1.end_bound()),
-            Some(core::cmp::Ordering::Greater)
-        );
-    }
-}
-
-pub fn ranges_intersect<T, R1, R2>(rhs: R1, lhs: R2) -> ()
-where
-    T: Ord,
-    R1: core::ops::RangeBounds<T>,
-    R2: core::ops::RangeBounds<T>,
-{
-    let a = rhs.start_bound();
-    let b = rhs.end_bound();
-    let x = lhs.start_bound();
-    let y = lhs.end_bound();
-
-    // check that a <=> x is different than b <=> x
-
-    // a <=> x
-    match a {
-        Bound::Included(a) => match x {
-            Bound::Included(x) => a.partial_cmp(x),
-            Bound::Excluded(x) => match a.partial_cmp(x) {
-                Some(Ordering::Equal) => Some(Ordering::Less),
-                ord => ord,
-            },
-            Bound::Unbounded => Some(Ordering::Greater),
-        },
-        Bound::Excluded(a) => match x {
-            Bound::Included(x) => match a.partial_cmp(x) {
-                Some(Ordering::Equal) => Some(Ordering::Greater),
-                ord => ord,
-            },
-            Bound::Excluded(x) => a.partial_cmp(x),
-            Bound::Unbounded => Some(Ordering::Less),
-        },
-        Bound::Unbounded => match x {
-            Bound::Unbounded => Some(Ordering::Equal),
-            _ => Some(Ordering::Less),
-        },
-    };
-
-    todo!()
-}

btrfs/src/v2/tree.rs

@@ -32,7 +32,7 @@ impl NodePtr {
     pub fn key_ptr(&self) -> &KeyPtr {
         match self {
             NodePtr::Unvisited(key) => key,
-            NodePtr::Visited { key, node } => key,
+            NodePtr::Visited { key, .. } => key,
         }
     }
 
@@ -216,7 +216,11 @@ pub mod entry {
             self.key
         }
 
-        pub fn item(&self) -> Result<TreeItem> {
+        pub fn item_and_value(&self) -> Result<(Item, TreeItem)> {
+            self.node.parse_item()
+        }
+
+        pub fn value(&self) -> Result<TreeItem> {
             Ok(self.node.parse_item()?.1)
         }
     }

btrfs/src/v2/volume.rs

@@ -1,5 +1,5 @@
 use core::mem::size_of;
-use core::ops::{Range, RangeBounds};
+use core::ops::RangeBounds;
 
 use alloc::collections::btree_map::Entry;
 use alloc::{collections::BTreeMap, rc::Rc, vec, vec::Vec};
@@ -490,13 +490,18 @@ impl<R: super::Read> Fs<R> {
             Some(crc as u64),
         );
 
-        if let Some((_, value)) = self.fs_root.find_key(&key)? {
+        match self.fs_root.entry(&key)? {
-            let dir_items = value.as_dir_item().expect("dir index");
+            super::tree::entry::Entry::Occupied(occupied) => {
-
+                let item = occupied
-            let item = dir_items.iter().find(|item| item.name() == child).cloned();
+                    .value()?
-            Ok(item)
+                    .as_dir_item()
-        } else {
+                    .expect("dir item")
-            Ok(None)
+                    .iter()
+                    .find(|item| item.name() == child)
+                    .cloned();
+                Ok(item)
+            }
+            super::tree::entry::Entry::Vacant(_) => Ok(None),
         }
     }
 
@@ -605,12 +610,13 @@ impl<R: super::Read> Fs<R> {
     fn find_inode_ref(&self, inode_id: u64) -> Result<Option<(Item, INodeRefEntry)>> {
         let key = PartialKey::new(Some(inode_id.into()), Some(ObjectType::INodeRef), None);
 
-        if let Some((item, value)) = self.fs_root.find_key(&key)? {
+        match self.fs_root.entry(&key)? {
-            let inode = value.as_inode_ref().expect("inoderef").clone();
+            super::tree::entry::Entry::Occupied(entry) => {
-
+                entry.item_and_value().map(|(item, value)| {
-            Ok(Some((item, inode)))
+                    Some((item, value.as_inode_ref().expect("inode ref").clone()))
-        } else {
+                })
-            Ok(None)
+            }
+            super::tree::entry::Entry::Vacant(_) => Ok(None),
         }
     }
 
@@ -626,22 +632,20 @@ impl<R: super::Read> Fs<R> {
             Some(inoderef.item().index.get()),
         );
 
-        if let Some((_, value)) = self.fs_root.find_key(&key)? {
+        match self.fs_root.entry(&key)? {
-            let dir_index = value.as_dir_index().expect("dir index").clone();
+            super::tree::entry::Entry::Occupied(entry) => entry
-            Ok(Some(dir_index))
+                .item_and_value()
-        } else {
+                .map(|(_, value)| Some(value.as_dir_index().expect("dir index").clone())),
-            Ok(None)
+            super::tree::entry::Entry::Vacant(_) => Ok(None),
         }
     }
 
     fn find_inode_item(&self, dir_item: &DirItemEntry) -> Result<Option<INodeItem>> {
-        dir_item.item().location;
+        match self.fs_root.entry(&dir_item.item().location)? {
-        if let Some((_, value)) = self.fs_root.find_key(&dir_item.item().location)? {
+            super::tree::entry::Entry::Occupied(entry) => entry
-            let inode = value.as_inode_item().expect("inode item").clone();
+                .item_and_value()
-
+                .map(|(_, value)| Some(value.as_inode_item().expect("inode item").clone())),
-            Ok(Some(inode))
+            super::tree::entry::Entry::Vacant(_) => Ok(None),
-        } else {
-            Ok(None)
         }
     }
 }
@@ -674,7 +678,7 @@ mod tests {
         let v2 = vol.into_volume2().expect("volume2");
 
         log::info!("roots:");
-        for (id, v) in v2.roots.iter() {
+        for (id, _) in v2.roots.iter() {
             log::info!("[{id:?}] ");
         }
     }
@@ -686,11 +690,11 @@ mod tests {
         let v2 = vol.into_volume2().expect("volume2");
 
         log::info!("roots:");
-        for (id, v) in v2.roots.iter() {
+        for (id, _) in v2.roots.iter() {
             log::info!("[{id:?}] ");
         }
         log::info!("roots rev:");
-        for (id, v) in v2.roots.iter().rev() {
+        for (id, _) in v2.roots.iter().rev() {
             log::info!("[{id:?}] ");
         }
     }
@@ -726,7 +730,7 @@ mod tests {
         let vol = Volume::new(file).expect("volume");
         let v2 = vol.into_volume2().expect("volume2");
 
-        let fs = v2.default_subvolume().expect("subvol");
+        _ = v2.default_subvolume().expect("subvol");
         Ok(())
     }
 
@@ -829,7 +833,7 @@ mod tests {
         log::info!("files 1:");
         let now = std::time::Instant::now();
         for (_id, entry) in fs.fs_root.iter() {
-            if let Some(dir) = entry.as_dir_index() {
+            if let Some(_dir) = entry.as_dir_index() {
                 //log::info!("{}", dir.name_as_string_lossy());
             }
         }
@@ -838,7 +842,7 @@ mod tests {
         log::info!("files 2:");
         let now = std::time::Instant::now();
         for (_id, entry) in fs.fs_root.iter() {
-            if let Some(dir) = entry.as_dir_index() {
+            if let Some(_dir) = entry.as_dir_index() {
                 //log::info!("{}", dir.name_as_string_lossy());
             }
         }