refactor(mmap): Add Error type for check_file_offset

This avoids duplicating all the variants across the unix/xen specific
error types.

Signed-off-by: Patrick Roy <[email protected]>

Author: roypat

src/mmap.rs

@@ -46,6 +46,23 @@ impl NewBitmap for () {
     fn with_len(_len: usize) -> Self {}
 }
 
+/// Errors that can occur during [`check_file_offset`]
+#[derive(Debug, thiserror::Error)]
+pub enum CheckFileOffsetError {
+    /// Seeking the end of the file returned an error.
+    #[error("Error seeking the end of the file: {0}")]
+    SeekEnd(std::io::Error),
+    /// Seeking the start of the file returned an error.
+    #[error("Error seeking the start of the file: {0}")]
+    SeekStart(std::io::Error),
+    /// A mapping with offset + length > EOF was attempted.
+    #[error("The specified file offset and length is greater then file length")]
+    MappingPastEof,
+    /// The specified file offset and length cause overflow when added.
+    #[error("The specified file offset and length cause overflow when added")]
+    InvalidOffsetLength,
+}
+
 // TODO: use this for Windows as well after we redefine the Error type there.
 #[cfg(unix)]
 /// Checks if a mapping of `size` bytes fits at the provided `file_offset`.
@@ -55,20 +72,20 @@ impl NewBitmap for () {
 pub fn check_file_offset(
     file_offset: &FileOffset,
     size: usize,
-) -> result::Result<(), MmapRegionError> {
+) -> result::Result<(), CheckFileOffsetError> {
     let mut file = file_offset.file();
     let start = file_offset.start();
 
     if let Some(end) = start.checked_add(size as u64) {
         let filesize = file
             .seek(SeekFrom::End(0))
-            .map_err(MmapRegionError::SeekEnd)?;
-        file.rewind().map_err(MmapRegionError::SeekStart)?;
+            .map_err(CheckFileOffsetError::SeekEnd)?;
+        file.rewind().map_err(CheckFileOffsetError::SeekStart)?;
         if filesize < end {
-            return Err(MmapRegionError::MappingPastEof);
+            return Err(CheckFileOffsetError::MappingPastEof);
         }
     } else {
-        return Err(MmapRegionError::InvalidOffsetLength);
+        return Err(CheckFileOffsetError::InvalidOffsetLength);
     }
 
     Ok(())

src/mmap_unix.rs

@@ -17,15 +17,12 @@ use std::result;
 
 use crate::bitmap::{Bitmap, BS};
 use crate::guest_memory::FileOffset;
-use crate::mmap::{check_file_offset, NewBitmap};
+use crate::mmap::{check_file_offset, CheckFileOffsetError, NewBitmap};
 use crate::volatile_memory::{self, VolatileMemory, VolatileSlice};
 
 /// Error conditions that may arise when creating a new `MmapRegion` object.
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
-    /// The specified file offset and length cause overflow when added.
-    #[error("The specified file offset and length cause overflow when added")]
-    InvalidOffsetLength,
     /// The specified pointer to the mapping is not page-aligned.
     #[error("The specified pointer to the mapping is not page-aligned")]
     InvalidPointer,
@@ -35,18 +32,12 @@ pub enum Error {
     /// Mappings using the same fd overlap in terms of file offset and length.
     #[error("Mappings using the same fd overlap in terms of file offset and length")]
     MappingOverlap,
-    /// A mapping with offset + length > EOF was attempted.
-    #[error("The specified file offset and length is greater then file length")]
-    MappingPastEof,
     /// The `mmap` call returned an error.
     #[error("{0}")]
     Mmap(io::Error),
-    /// Seeking the end of the file returned an error.
-    #[error("Error seeking the end of the file: {0}")]
-    SeekEnd(io::Error),
-    /// Seeking the start of the file returned an error.
-    #[error("Error seeking the start of the file: {0}")]
-    SeekStart(io::Error),
+    /// Error validating a [`FileOffset`]
+    #[error("{0}")]
+    ValidateFile(#[from] CheckFileOffsetError),
 }
 
 pub type Result<T> = result::Result<T, Error>;
@@ -558,7 +549,10 @@ mod tests {
             prot,
             flags,
         );
-        assert!(matches!(r.unwrap_err(), Error::InvalidOffsetLength));
+        assert!(matches!(
+            r.unwrap_err(),
+            Error::ValidateFile(CheckFileOffsetError::InvalidOffsetLength)
+        ));
 
         // Offset + size is greater than the size of the file (which is 0 at this point).
         let r = MmapRegion::build(
@@ -567,7 +561,10 @@ mod tests {
             prot,
             flags,
         );
-        assert!(matches!(r.unwrap_err(), Error::MappingPastEof));
+        assert!(matches!(
+            r.unwrap_err(),
+            Error::ValidateFile(CheckFileOffsetError::MappingPastEof)
+        ));
 
         // MAP_FIXED was specified among the flags.
         let r = MmapRegion::build(

src/mmap_xen.rs

@@ -26,30 +26,18 @@ use tests::ioctl_with_ref;
 
 use crate::bitmap::{Bitmap, BS};
 use crate::guest_memory::{FileOffset, GuestAddress};
-use crate::mmap::{check_file_offset, NewBitmap};
+use crate::mmap::{check_file_offset, CheckFileOffsetError, NewBitmap};
 use crate::volatile_memory::{self, VolatileMemory, VolatileSlice};
 
 /// Error conditions that may arise when creating a new `MmapRegion` object.
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
-    /// The specified file offset and length cause overflow when added.
-    #[error("The specified file offset and length cause overflow when added")]
-    InvalidOffsetLength,
     /// The forbidden `MAP_FIXED` flag was specified.
     #[error("The forbidden `MAP_FIXED` flag was specified")]
     MapFixed,
-    /// A mapping with offset + length > EOF was attempted.
-    #[error("The specified file offset and length is greater then file length")]
-    MappingPastEof,
     /// The `mmap` call returned an error.
     #[error("{0}")]
     Mmap(io::Error),
-    /// Seeking the end of the file returned an error.
-    #[error("Error seeking the end of the file: {0}")]
-    SeekEnd(io::Error),
-    /// Seeking the start of the file returned an error.
-    #[error("Error seeking the start of the file: {0}")]
-    SeekStart(io::Error),
     /// Invalid file offset.
     #[error("Invalid file offset")]
     InvalidFileOffset,
@@ -65,6 +53,9 @@ pub enum Error {
     /// Unexpected error.
     #[error("Unexpected error")]
     UnexpectedError,
+    /// Error validating a [`FileOffset`]
+    #[error("{0}")]
+    ValidateFile(#[from] CheckFileOffsetError),
 }
 
 type Result<T> = result::Result<T, Error>;
@@ -497,15 +488,15 @@ fn pages(size: usize) -> (usize, usize) {
 fn validate_file(file_offset: &Option<FileOffset>) -> Result<(i32, u64)> {
     let file_offset = match file_offset {
         Some(f) => f,
-        None => return Err(Error::InvalidFileOffset),
+        None => return Err(CheckFileOffsetError::InvalidOffsetLength.into()),
     };
 
     let fd = file_offset.file().as_raw_fd();
     let f_offset = file_offset.start();
 
     // We don't allow file offsets with Xen foreign mappings.
     if f_offset != 0 {
-        return Err(Error::InvalidOffsetLength);
+        return Err(CheckFileOffsetError::InvalidOffsetLength.into());
     }
 
     Ok((fd, f_offset))
@@ -1134,7 +1125,10 @@ mod tests {
         let mut range = MmapRange::initialized(true);
         range.file_offset = Some(FileOffset::new(TempFile::new().unwrap().into_file(), 1));
         let r = MmapXenForeign::new(&range);
-        assert!(matches!(r.unwrap_err(), Error::InvalidOffsetLength));
+        assert!(matches!(
+            r.unwrap_err(),
+            Error::ValidateFile(CheckFileOffsetError::InvalidOffsetLength)
+        ));
 
         let mut range = MmapRange::initialized(true);
         range.size = 0;
@@ -1171,7 +1165,10 @@ mod tests {
         let mut range = MmapRange::initialized(true);
         range.file_offset = Some(FileOffset::new(TempFile::new().unwrap().into_file(), 1));
         let r = MmapXenGrant::new(&range, MmapXenFlags::NO_ADVANCE_MAP);
-        assert!(matches!(r.unwrap_err(), Error::InvalidOffsetLength));
+        assert!(matches!(
+            r.unwrap_err(),
+            Error::ValidateFile(CheckFileOffsetError::InvalidOffsetLength)
+        ));
 
         let mut range = MmapRange::initialized(true);
         range.size = 0;