Use InodeIndex instead of u32 in CorruptKind variants

The interesting change is in error.rs, changing all of the `/// Inode number.` fields to use the `InodeIndex` type. Also removed inner docstrings from those fields, since the type name is sufficiently clear. The rest of the changes are mechanical compile/lint fixes.
nicholasbishop · Jan 29, 2025 · 571d7ad · 571d7ad
1 parent bc7a3b6
commit 571d7ad
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 84 deletions.
diff --git a/src/dir_block.rs b/src/dir_block.rs
@@ -73,7 +73,7 @@ impl DirBlock<'_> {
         if actual_checksum.finalize() == expected_checksum {
             Ok(())
         } else {
-            Err(CorruptKind::DirBlockChecksum(self.dir_inode.get()).into())
+            Err(CorruptKind::DirBlockChecksum(self.dir_inode).into())
         }
     }
 

diff --git a/src/dir_entry.rs b/src/dir_entry.rs
@@ -237,7 +237,7 @@ impl DirEntry {
     ) -> Result<(Option<Self>, usize), Ext4Error> {
         const NAME_OFFSET: usize = 8;
 
-        let err = || CorruptKind::DirEntry(inode.get()).into();
+        let err = || CorruptKind::DirEntry(inode).into();
 
         // Check size (the full entry will usually be larger than this),
         // but these header fields must be present.
@@ -492,7 +492,10 @@ mod tests {
         // Error: not enough data.
         let err = DirEntry::from_bytes(fs.clone(), &[], inode1, path.clone())
             .unwrap_err();
-        assert_eq!(*err.as_corrupt().unwrap(), CorruptKind::DirEntry(1).into());
+        assert_eq!(
+            *err.as_corrupt().unwrap(),
+            CorruptKind::DirEntry(inode1).into()
+        );
 
         // Error: not enough data for the name.
         let mut bytes = Vec::new();

diff --git a/src/dir_htree.rs b/src/dir_htree.rs
@@ -89,7 +89,7 @@ impl<'a> InternalNode<'a> {
     /// Create an `InternalNode` from raw bytes. These bytes come from a
     /// directory block, see [`from_root_block`] and [`from_non_root_block`].
     fn new(mut bytes: &'a [u8], inode: InodeIndex) -> Result<Self, Ext4Error> {
-        let err = CorruptKind::DirEntry(inode.get()).into();
+        let err = CorruptKind::DirEntry(inode).into();
 
         // At least the header entry must be present.
         if bytes.len() < Self::ENTRY_SIZE {
@@ -177,7 +177,7 @@ fn read_root_block(
     // Get the first block.
     let block_index = file_blocks
         .next()
-        .ok_or_else(|| CorruptKind::DirEntry(inode.index.get()))??;
+        .ok_or(CorruptKind::DirEntry(inode.index))??;
 
     // Read the first block of the extent.
     let dir_block = DirBlock {
@@ -204,7 +204,7 @@ fn read_dot_or_dotdot(
     name: DirEntryName<'_>,
     block: &[u8],
 ) -> Result<Option<DirEntry>, Ext4Error> {
-    let corrupt = || CorruptKind::DirEntry(inode.index.get()).into();
+    let corrupt = || CorruptKind::DirEntry(inode.index).into();
 
     let offset = if name == "." {
         0
@@ -240,13 +240,13 @@ fn find_extent_for_block(
         let start = extent.block_within_file;
         let end = start
             .checked_add(u32::from(extent.num_blocks))
-            .ok_or(CorruptKind::DirEntry(inode.index.get()))?;
+            .ok_or(CorruptKind::DirEntry(inode.index))?;
         if block >= start && block < end {
             return Ok(extent);
         }
     }
 
-    Err(CorruptKind::DirEntry(inode.index.get()).into())
+    Err(CorruptKind::DirEntry(inode.index).into())
 }
 
 /// Convert from a block offset within a file to an absolute block index.
@@ -259,17 +259,17 @@ fn block_from_file_block(
         let extent = find_extent_for_block(fs, inode, relative_block)?;
         let block_within_extent = relative_block
             .checked_sub(extent.block_within_file)
-            .ok_or(CorruptKind::DirEntry(inode.index.get()))?;
+            .ok_or(CorruptKind::DirEntry(inode.index))?;
         let absolute_block = extent
             .start_block
             .checked_add(u64::from(block_within_extent))
-            .ok_or(CorruptKind::DirEntry(inode.index.get()))?;
+            .ok_or(CorruptKind::DirEntry(inode.index))?;
         Ok(absolute_block)
     } else {
         let mut block_map = FileBlocks::new(fs.clone(), inode)?;
         block_map
             .nth(usize_from_u32(relative_block))
-            .ok_or_else(|| CorruptKind::DirEntry(inode.index.get()))?
+            .ok_or(CorruptKind::DirEntry(inode.index))?
     }
 }
 
@@ -302,7 +302,7 @@ fn find_leaf_node(
     let hash = dir_hash_md4_half(name, &fs.0.superblock.htree_hash_seed);
     let mut child_block_relative = root_node
         .lookup_block_by_hash(hash)
-        .ok_or(CorruptKind::DirEntry(inode.index.get()))?;
+        .ok_or(CorruptKind::DirEntry(inode.index))?;
 
     // Descend through the tree one level at a time. The first iteration
     // of the loop goes from the root node to a child. The last
@@ -330,7 +330,7 @@ fn find_leaf_node(
                 InternalNode::from_non_root_block(block, inode.index)?;
             child_block_relative = inner_node
                 .lookup_block_by_hash(hash)
-                .ok_or(CorruptKind::DirEntry(inode.index.get()))?;
+                .ok_or(CorruptKind::DirEntry(inode.index))?;
         }
     }
 
@@ -384,7 +384,7 @@ pub(crate) fn get_dir_entry_via_htree(
         )?;
         offset_within_block = offset_within_block
             .checked_add(entry_size)
-            .ok_or(CorruptKind::DirEntry(inode.index.get()))?;
+            .ok_or(CorruptKind::DirEntry(inode.index))?;
         let Some(dir_entry) = dir_entry else {
             continue;
         };

diff --git a/src/error.rs b/src/error.rs
@@ -7,6 +7,7 @@
 // except according to those terms.
 
 use crate::features::IncompatibleFeatures;
+use crate::inode::InodeIndex;
 use alloc::boxed::Box;
 use core::error::Error;
 use core::fmt::{self, Debug, Display, Formatter};
@@ -227,74 +228,41 @@ pub(crate) enum CorruptKind {
     ),
 
     /// An inode's checksum is invalid.
-    InodeChecksum(
-        /// Inode number.
-        u32,
-    ),
+    InodeChecksum(InodeIndex),
 
     /// An inode is invalid.
-    Inode(
-        /// Inode number.
-        u32,
-    ),
+    Inode(InodeIndex),
 
     /// The target of a symlink is not a valid path.
-    SymlinkTarget(
-        /// Inode number.
-        u32,
-    ),
+    SymlinkTarget(InodeIndex),
 
     /// The number of blocks in a file exceeds 2^32.
     TooManyBlocksInFile,
 
     /// An extent's magic is invalid.
-    ExtentMagic(
-        /// Inode number.
-        u32,
-    ),
+    ExtentMagic(InodeIndex),
 
     /// An extent's checksum is invalid.
-    ExtentChecksum(
-        /// Inode number.
-        u32,
-    ),
+    ExtentChecksum(InodeIndex),
 
     /// An extent's depth is greater than five.
-    ExtentDepth(
-        /// Inode number.
-        u32,
-    ),
+    ExtentDepth(InodeIndex),
 
     /// Not enough data is present to read an extent node.
-    ExtentNotEnoughData(
-        /// Inode number.
-        u32,
-    ),
+    ExtentNotEnoughData(InodeIndex),
 
     /// An extent points to an invalid block.
-    ExtentBlock(
-        /// Inode number.
-        u32,
-    ),
+    ExtentBlock(InodeIndex),
 
     /// An extent node's size exceeds the block size.
-    ExtentNodeSize(
-        /// Inode number.
-        u32,
-    ),
+    ExtentNodeSize(InodeIndex),
 
     /// A directory block's checksum is invalid.
-    DirBlockChecksum(
-        /// Inode number.
-        u32,
-    ),
+    DirBlockChecksum(InodeIndex),
 
     // TODO: consider breaking this down into more specific problems.
     /// A directory entry is invalid.
-    DirEntry(
-        /// Inode number.
-        u32,
-    ),
+    DirEntry(InodeIndex),
 
     /// Invalid read of a block.
     BlockRead {

diff --git a/src/inode.rs b/src/inode.rs
@@ -128,7 +128,7 @@ impl Inode {
         data: &[u8],
     ) -> Result<(Self, u32), Ext4Error> {
         if data.len() < (Self::I_CHECKSUM_HI_OFFSET + 2) {
-            return Err(CorruptKind::Inode(index.get()).into());
+            return Err(CorruptKind::Inode(index).into());
         }
 
         let i_mode = read_u16le(data, 0x0);
@@ -176,7 +176,7 @@ impl Inode {
                     uid,
                     gid,
                     file_type: FileType::try_from(mode)
-                        .map_err(|_| CorruptKind::Inode(index.get()))?,
+                        .map_err(|_| CorruptKind::Inode(index))?,
                 },
                 flags: InodeFlags::from_bits_retain(i_flags),
                 checksum_base,
@@ -192,8 +192,7 @@ impl Inode {
         inode: InodeIndex,
     ) -> Result<Self, Ext4Error> {
         let (block_index, offset_within_block) =
-            get_inode_location(ext4, inode)
-                .ok_or(CorruptKind::Inode(inode.get()))?;
+            get_inode_location(ext4, inode).ok_or(CorruptKind::Inode(inode))?;
 
         let mut data = vec![0; usize::from(ext4.0.superblock.inode_size)];
         ext4.read_from_block(block_index, offset_within_block, &mut data)?;
@@ -227,9 +226,7 @@ impl Inode {
 
             let actual_checksum = checksum.finalize();
             if actual_checksum != expected_checksum {
-                return Err(
-                    CorruptKind::InodeChecksum(inode.index.get()).into()
-                );
+                return Err(CorruptKind::InodeChecksum(inode.index).into());
             }
         }
 
@@ -246,7 +243,7 @@ impl Inode {
 
         // An empty symlink target is not allowed.
         if self.metadata.size_in_bytes == 0 {
-            return Err(CorruptKind::SymlinkTarget(self.index.get()).into());
+            return Err(CorruptKind::SymlinkTarget(self.index).into());
         }
 
         // Symlink targets of up to 59 bytes are stored inline. Longer
@@ -258,14 +255,12 @@ impl Inode {
             let len = usize::try_from(self.metadata.size_in_bytes).unwrap();
             let target = &self.inline_data[..len];
 
-            PathBuf::try_from(target).map_err(|_| {
-                CorruptKind::SymlinkTarget(self.index.get()).into()
-            })
+            PathBuf::try_from(target)
+                .map_err(|_| CorruptKind::SymlinkTarget(self.index).into())
         } else {
             let data = ext4.read_inode_file(self)?;
-            PathBuf::try_from(data).map_err(|_| {
-                CorruptKind::SymlinkTarget(self.index.get()).into()
-            })
+            PathBuf::try_from(data)
+                .map_err(|_| CorruptKind::SymlinkTarget(self.index).into())
         }
     }
 

diff --git a/src/iters/extents.rs b/src/iters/extents.rs
@@ -73,7 +73,7 @@ impl NodeHeader {
     /// Read a `NodeHeader` from a byte slice.
     fn from_bytes(data: &[u8], inode: InodeIndex) -> Result<Self, Ext4Error> {
         if data.len() < ENTRY_SIZE_IN_BYTES {
-            return Err(CorruptKind::ExtentNotEnoughData(inode.get()).into());
+            return Err(CorruptKind::ExtentNotEnoughData(inode).into());
         }
 
         let eh_magic = read_u16le(data, 0);
@@ -82,11 +82,11 @@ impl NodeHeader {
         let eh_depth = read_u16le(data, 6);
 
         if eh_magic != 0xf30a {
-            return Err(CorruptKind::ExtentMagic(inode.get()).into());
+            return Err(CorruptKind::ExtentMagic(inode).into());
         }
 
         if eh_depth > 5 {
-            return Err(CorruptKind::ExtentDepth(inode.get()).into());
+            return Err(CorruptKind::ExtentDepth(inode).into());
         }
 
         Ok(Self {
@@ -119,7 +119,7 @@ impl ToVisitItem {
         // The node data must be large enough to contain the number of
         // entries specified in the header.
         if node.len() < header.node_size_in_bytes() {
-            return Err(CorruptKind::ExtentNotEnoughData(inode.get()).into());
+            return Err(CorruptKind::ExtentNotEnoughData(inode).into());
         }
 
         // Remove unused data at the end (e.g. checksum data).
@@ -251,9 +251,7 @@ impl Extents {
                 checksum_offset.checked_add(checksum_size).unwrap();
             // Extent nodes are not allowed to exceed the block size.
             if child_node_size > self.ext4.0.superblock.block_size {
-                return Err(
-                    CorruptKind::ExtentNodeSize(self.inode.get()).into()
-                );
+                return Err(CorruptKind::ExtentNodeSize(self.inode).into());
             }
             let mut child_node = vec![0; child_node_size];
             self.ext4.read_from_block(child_block, 0, &mut child_node)?;
@@ -269,9 +267,7 @@ impl Extents {
                 checksum.update(&child_node[..checksum_offset]);
                 let actual_checksum = checksum.finalize();
                 if expected_checksum != actual_checksum {
-                    return Err(
-                        CorruptKind::ExtentChecksum(self.inode.get()).into()
-                    );
+                    return Err(CorruptKind::ExtentChecksum(self.inode).into());
                 }
             }
 

diff --git a/src/iters/file_blocks/extents_blocks.rs b/src/iters/file_blocks/extents_blocks.rs
@@ -148,7 +148,7 @@ impl ExtentsBlocks {
         let block = extent
             .start_block
             .checked_add(u64::from(self.block_within_extent))
-            .ok_or(CorruptKind::ExtentBlock(self.inode.get()))?;
+            .ok_or(CorruptKind::ExtentBlock(self.inode))?;
 
         // OK to unwrap: `block_within_extent` is less than `num_blocks`
         // (checked above) so adding `1` cannot fail.

diff --git a/src/iters/read_dir.rs b/src/iters/read_dir.rs
@@ -144,7 +144,7 @@ impl ReadDir {
         self.offset_within_block = self
             .offset_within_block
             .checked_add(entry_size)
-            .ok_or(CorruptKind::DirEntry(self.inode.get()))?;
+            .ok_or(CorruptKind::DirEntry(self.inode))?;
 
         Ok(entry)
     }