Use custom error types instead of ()

This fixes the clippy warnings we currently see on CI. Since this changes the signature of public functions, this is a **breaking change**. However, I don't expect that much code is broken by this.
rust-osdev · Dec 28, 2020 · f2039dd · f2039dd
1 parent 5b0eac1
commit f2039dd
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 22 deletions.
diff --git a/src/structures/paging/frame.rs b/src/structures/paging/frame.rs
@@ -1,5 +1,6 @@
 //! Abstractions for default-sized and huge physical memory frames.
 
+use super::page::AddressNotAligned;
 use crate::structures::paging::page::{PageSize, Size4KiB};
 use crate::PhysAddr;
 use core::fmt;
@@ -19,9 +20,9 @@ impl<S: PageSize> PhysFrame<S> {
     ///
     /// Returns an error if the address is not correctly aligned (i.e. is not a valid frame start).
     #[inline]
-    pub fn from_start_address(address: PhysAddr) -> Result<Self, ()> {
+    pub fn from_start_address(address: PhysAddr) -> Result<Self, AddressNotAligned> {
         if !address.is_aligned(S::SIZE) {
-            return Err(());
+            return Err(AddressNotAligned);
         }
         Ok(PhysFrame::containing_address(address))
     }

diff --git a/src/structures/paging/mapper/mapped_page_table.rs b/src/structures/paging/mapper/mapped_page_table.rs
@@ -2,7 +2,7 @@ use crate::structures::paging::{
     frame::PhysFrame,
     frame_alloc::FrameAllocator,
     mapper::*,
-    page::{Page, Size1GiB, Size2MiB, Size4KiB},
+    page::{AddressNotAligned, Page, Size1GiB, Size2MiB, Size4KiB},
     page_table::{FrameError, PageTable, PageTableEntry, PageTableFlags},
 };
 
@@ -178,7 +178,7 @@ impl<'a, P: PhysToVirt> Mapper<Size1GiB> for MappedPageTable<'a, P> {
         }
 
         let frame = PhysFrame::from_start_address(p3_entry.addr())
-            .map_err(|()| UnmapError::InvalidFrameAddress(p3_entry.addr()))?;
+            .map_err(|AddressNotAligned| UnmapError::InvalidFrameAddress(p3_entry.addr()))?;
 
         p3_entry.set_unused();
         Ok((frame, MapperFlush::new(page)))
@@ -246,7 +246,7 @@ impl<'a, P: PhysToVirt> Mapper<Size1GiB> for MappedPageTable<'a, P> {
         }
 
         PhysFrame::from_start_address(p3_entry.addr())
-            .map_err(|()| TranslateError::InvalidFrameAddress(p3_entry.addr()))
+            .map_err(|AddressNotAligned| TranslateError::InvalidFrameAddress(p3_entry.addr()))
     }
 }
 
@@ -289,7 +289,7 @@ impl<'a, P: PhysToVirt> Mapper<Size2MiB> for MappedPageTable<'a, P> {
         }
 
         let frame = PhysFrame::from_start_address(p2_entry.addr())
-            .map_err(|()| UnmapError::InvalidFrameAddress(p2_entry.addr()))?;
+            .map_err(|AddressNotAligned| UnmapError::InvalidFrameAddress(p2_entry.addr()))?;
 
         p2_entry.set_unused();
         Ok((frame, MapperFlush::new(page)))
@@ -374,7 +374,7 @@ impl<'a, P: PhysToVirt> Mapper<Size2MiB> for MappedPageTable<'a, P> {
         }
 
         PhysFrame::from_start_address(p2_entry.addr())
-            .map_err(|()| TranslateError::InvalidFrameAddress(p2_entry.addr()))
+            .map_err(|AddressNotAligned| TranslateError::InvalidFrameAddress(p2_entry.addr()))
     }
 }
 
@@ -518,7 +518,7 @@ impl<'a, P: PhysToVirt> Mapper<Size4KiB> for MappedPageTable<'a, P> {
         }
 
         PhysFrame::from_start_address(p1_entry.addr())
-            .map_err(|()| TranslateError::InvalidFrameAddress(p1_entry.addr()))
+            .map_err(|AddressNotAligned| TranslateError::InvalidFrameAddress(p1_entry.addr()))
     }
 }
 
@@ -572,7 +572,7 @@ impl<'a, P: PhysToVirt> MapperAllSizes for MappedPageTable<'a, P> {
 
         let frame = match PhysFrame::from_start_address(p1_entry.addr()) {
             Ok(frame) => frame,
-            Err(()) => return TranslateResult::InvalidFrameAddress(p1_entry.addr()),
+            Err(AddressNotAligned) => return TranslateResult::InvalidFrameAddress(p1_entry.addr()),
         };
         let offset = u64::from(addr.page_offset());
         let flags = p1_entry.flags();

diff --git a/src/structures/paging/mapper/mod.rs b/src/structures/paging/mapper/mod.rs
@@ -4,7 +4,7 @@ pub use self::mapped_page_table::{MappedPageTable, PhysToVirt};
 #[cfg(target_pointer_width = "64")]
 pub use self::offset_page_table::OffsetPageTable;
 #[cfg(feature = "instructions")]
-pub use self::recursive_page_table::RecursivePageTable;
+pub use self::recursive_page_table::{InvalidPageTable, RecursivePageTable};
 
 use crate::structures::paging::{
     frame_alloc::FrameAllocator, page_table::PageTableFlags, Page, PageSize, PhysFrame, Size1GiB,

diff --git a/src/structures/paging/mapper/recursive_page_table.rs b/src/structures/paging/mapper/recursive_page_table.rs
@@ -1,11 +1,13 @@
 //! Access the page tables through a recursively mapped level 4 table.
 
+use core::fmt;
+
 use super::*;
 use crate::registers::control::Cr3;
 use crate::structures::paging::PageTableIndex;
 use crate::structures::paging::{
     frame_alloc::FrameAllocator,
-    page::NotGiantPageSize,
+    page::{AddressNotAligned, NotGiantPageSize},
     page_table::{FrameError, PageTable, PageTableEntry, PageTableFlags},
     Page, PageSize, PhysFrame, Size1GiB, Size2MiB, Size4KiB,
 };
@@ -46,18 +48,18 @@ impl<'a> RecursivePageTable<'a> {
     ///
     /// Otherwise `Err(())` is returned.
     #[inline]
-    pub fn new(table: &'a mut PageTable) -> Result<Self, ()> {
+    pub fn new(table: &'a mut PageTable) -> Result<Self, InvalidPageTable> {
         let page = Page::containing_address(VirtAddr::new(table as *const _ as u64));
         let recursive_index = page.p4_index();
 
         if page.p3_index() != recursive_index
             || page.p2_index() != recursive_index
             || page.p1_index() != recursive_index
         {
-            return Err(());
+            return Err(InvalidPageTable::NotRecursive);
         }
         if Ok(Cr3::read().0) != table[recursive_index].frame() {
-            return Err(());
+            return Err(InvalidPageTable::NotActive);
         }
 
         Ok(RecursivePageTable {
@@ -326,7 +328,7 @@ impl<'a> Mapper<Size1GiB> for RecursivePageTable<'a> {
         }
 
         let frame = PhysFrame::from_start_address(p3_entry.addr())
-            .map_err(|()| UnmapError::InvalidFrameAddress(p3_entry.addr()))?;
+            .map_err(|AddressNotAligned| UnmapError::InvalidFrameAddress(p3_entry.addr()))?;
 
         p3_entry.set_unused();
         Ok((frame, MapperFlush::new(page)))
@@ -404,7 +406,7 @@ impl<'a> Mapper<Size1GiB> for RecursivePageTable<'a> {
         }
 
         PhysFrame::from_start_address(p3_entry.addr())
-            .map_err(|()| TranslateError::InvalidFrameAddress(p3_entry.addr()))
+            .map_err(|AddressNotAligned| TranslateError::InvalidFrameAddress(p3_entry.addr()))
     }
 }
 
@@ -454,7 +456,7 @@ impl<'a> Mapper<Size2MiB> for RecursivePageTable<'a> {
         }
 
         let frame = PhysFrame::from_start_address(p2_entry.addr())
-            .map_err(|()| UnmapError::InvalidFrameAddress(p2_entry.addr()))?;
+            .map_err(|AddressNotAligned| UnmapError::InvalidFrameAddress(p2_entry.addr()))?;
 
         p2_entry.set_unused();
         Ok((frame, MapperFlush::new(page)))
@@ -561,7 +563,7 @@ impl<'a> Mapper<Size2MiB> for RecursivePageTable<'a> {
         }
 
         PhysFrame::from_start_address(p2_entry.addr())
-            .map_err(|()| TranslateError::InvalidFrameAddress(p2_entry.addr()))
+            .map_err(|AddressNotAligned| TranslateError::InvalidFrameAddress(p2_entry.addr()))
     }
 }
 
@@ -752,7 +754,7 @@ impl<'a> Mapper<Size4KiB> for RecursivePageTable<'a> {
         }
 
         PhysFrame::from_start_address(p1_entry.addr())
-            .map_err(|()| TranslateError::InvalidFrameAddress(p1_entry.addr()))
+            .map_err(|AddressNotAligned| TranslateError::InvalidFrameAddress(p1_entry.addr()))
     }
 }
 
@@ -815,7 +817,7 @@ impl<'a> MapperAllSizes for RecursivePageTable<'a> {
 
         let frame = match PhysFrame::from_start_address(p1_entry.addr()) {
             Ok(frame) => frame,
-            Err(()) => return TranslateResult::InvalidFrameAddress(p1_entry.addr()),
+            Err(AddressNotAligned) => return TranslateResult::InvalidFrameAddress(p1_entry.addr()),
         };
         let offset = u64::from(addr.page_offset());
         let flags = p1_entry.flags();
@@ -827,6 +829,33 @@ impl<'a> MapperAllSizes for RecursivePageTable<'a> {
     }
 }
 
+/// The given page table was not suitable to create a `RecursivePageTable`.
+#[derive(Debug)]
+pub enum InvalidPageTable {
+    /// The given page table was not at an recursive address.
+    ///
+    /// The page table address must be of the form `0o_xxx_xxx_xxx_xxx_0000` where `xxx`
+    /// is the recursive entry.
+    NotRecursive,
+    /// The given page table was not active on the CPU.
+    ///
+    /// The recursive page table design requires that the given level 4 table is active
+    /// on the CPU because otherwise it's not possible to access the other page tables
+    /// through recursive memory addresses.
+    NotActive,
+}
+
+impl fmt::Display for InvalidPageTable {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            InvalidPageTable::NotRecursive => {
+                write!(f, "given page table address is not recursive")
+            }
+            InvalidPageTable::NotActive => write!(f, "given page table is not active on the CPU"),
+        }
+    }
+}
+
 #[inline]
 fn p3_ptr<S: PageSize>(page: Page<S>, recursive_index: PageTableIndex) -> *mut PageTable {
     p3_page(page, recursive_index).start_address().as_mut_ptr()

diff --git a/src/structures/paging/page.rs b/src/structures/paging/page.rs
@@ -67,9 +67,9 @@ impl<S: PageSize> Page<S> {
     ///
     /// Returns an error if the address is not correctly aligned (i.e. is not a valid page start).
     #[inline]
-    pub fn from_start_address(address: VirtAddr) -> Result<Self, ()> {
+    pub fn from_start_address(address: VirtAddr) -> Result<Self, AddressNotAligned> {
         if !address.is_aligned(S::SIZE) {
-            return Err(());
+            return Err(AddressNotAligned);
         }
         Ok(Page::containing_address(address))
     }
@@ -362,6 +362,16 @@ impl<S: PageSize> fmt::Debug for PageRangeInclusive<S> {
     }
 }
 
+/// The given address was not sufficiently aligned.
+#[derive(Debug)]
+pub struct AddressNotAligned;
+
+impl fmt::Display for AddressNotAligned {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "the given address was not sufficiently aligned")
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;