diff --git a/CHANGELOG.md b/CHANGELOG.md index 1881e7bb..6934c352 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Added - \[[#311](https://github.com/rust-vmm/vm-memory/pull/311)\] Allow compiling without the ReadVolatile and WriteVolatile implementations +- \[[#312](https://github.com/rust-vmm/vm-memory/pull/312)\] `GuestRegionContainer`, a generic container of `GuestMemoryRegion`s, generalizing `GuestMemoryMmap` (which + is now a type alias for `GuestRegionContainer`) ### Changed diff --git a/src/guest_memory.rs b/src/guest_memory.rs index 189ea8dd..c45d764c 100644 --- a/src/guest_memory.rs +++ b/src/guest_memory.rs @@ -408,7 +408,10 @@ pub trait GuestMemory { fn num_regions(&self) -> usize; /// Returns the region containing the specified address or `None`. - fn find_region(&self, addr: GuestAddress) -> Option<&Self::R>; + fn find_region(&self, addr: GuestAddress) -> Option<&Self::R> { + self.iter() + .find(|region| addr >= region.start_addr() && addr <= region.last_addr()) + } /// Gets an iterator over the entries in the collection. /// @@ -584,6 +587,168 @@ pub trait GuestMemory { } } +/// Errors that can occur when creating a memory map. +#[derive(Debug, thiserror::Error)] +pub enum GuestRegionCollectionError { + /// Adding the guest base address to the length of the underlying mapping resulted + /// in an overflow. + #[error("Adding the guest base address to the length of the underlying mapping resulted in an overflow")] + #[cfg(feature = "backend-mmap")] + InvalidGuestRegion, + /// Error creating a `MmapRegion` object. + #[error("{0}")] + #[cfg(feature = "backend-mmap")] + MmapRegion(crate::mmap::MmapRegionError), + /// No memory region found. + #[error("No memory region found")] + NoMemoryRegion, + /// Some of the memory regions intersect with each other. + #[error("Some of the memory regions intersect with each other")] + MemoryRegionOverlap, + /// The provided memory regions haven't been sorted. + #[error("The provided memory regions haven't been sorted")] + UnsortedMemoryRegions, +} + +/// [`GuestMemory`](trait.GuestMemory.html) implementation based on a homogeneous collection +/// of [`GuestMemoryRegion`] implementations. +/// +/// Represents a sorted set of non-overlapping physical guest memory regions. +#[derive(Debug)] +pub struct GuestRegionCollection { + regions: Vec>, +} + +impl Default for GuestRegionCollection { + fn default() -> Self { + Self { + regions: Vec::new(), + } + } +} + +impl Clone for GuestRegionCollection { + fn clone(&self) -> Self { + GuestRegionCollection { + regions: self.regions.iter().map(Arc::clone).collect(), + } + } +} + +impl GuestRegionCollection { + /// Creates an empty `GuestMemoryMmap` instance. + pub fn new() -> Self { + Self::default() + } + + /// Creates a new [`GuestRegionCollection`] from a vector of regions. + /// + /// # Arguments + /// + /// * `regions` - The vector of regions. + /// The regions shouldn't overlap, and they should be sorted + /// by the starting address. + pub fn from_regions( + mut regions: Vec, + ) -> std::result::Result { + Self::from_arc_regions(regions.drain(..).map(Arc::new).collect()) + } + + /// Creates a new [`GuestRegionCollection`] from a vector of Arc regions. + /// + /// Similar to the constructor `from_regions()` as it returns a + /// [`GuestRegionCollection`]. The need for this constructor is to provide a way for + /// consumer of this API to create a new [`GuestRegionCollection`] based on existing + /// regions coming from an existing [`GuestRegionCollection`] instance. + /// + /// # Arguments + /// + /// * `regions` - The vector of `Arc` regions. + /// The regions shouldn't overlap and they should be sorted + /// by the starting address. + pub fn from_arc_regions( + regions: Vec>, + ) -> std::result::Result { + if regions.is_empty() { + return Err(GuestRegionCollectionError::NoMemoryRegion); + } + + for window in regions.windows(2) { + let prev = &window[0]; + let next = &window[1]; + + if prev.start_addr() > next.start_addr() { + return Err(GuestRegionCollectionError::UnsortedMemoryRegions); + } + + if prev.last_addr() >= next.start_addr() { + return Err(GuestRegionCollectionError::MemoryRegionOverlap); + } + } + + Ok(Self { regions }) + } + + /// Insert a region into the `GuestMemoryMmap` object and return a new `GuestMemoryMmap`. + /// + /// # Arguments + /// * `region`: the memory region to insert into the guest memory object. + pub fn insert_region( + &self, + region: Arc, + ) -> std::result::Result, GuestRegionCollectionError> { + let mut regions = self.regions.clone(); + regions.push(region); + regions.sort_by_key(|x| x.start_addr()); + + Self::from_arc_regions(regions) + } + + /// Remove a region from the [`GuestRegionCollection`] object and return a new `GuestRegionCollection` + /// on success, together with the removed region. + /// + /// # Arguments + /// * `base`: base address of the region to be removed + /// * `size`: size of the region to be removed + pub fn remove_region( + &self, + base: GuestAddress, + size: GuestUsize, + ) -> std::result::Result<(GuestRegionCollection, Arc), GuestRegionCollectionError> { + if let Ok(region_index) = self.regions.binary_search_by_key(&base, |x| x.start_addr()) { + if self.regions.get(region_index).unwrap().len() == size { + let mut regions = self.regions.clone(); + let region = regions.remove(region_index); + return Ok((Self { regions }, region)); + } + } + + Err(GuestRegionCollectionError::NoMemoryRegion) + } +} + +impl GuestMemory for GuestRegionCollection { + type R = R; + + fn num_regions(&self) -> usize { + self.regions.len() + } + + fn find_region(&self, addr: GuestAddress) -> Option<&R> { + let index = match self.regions.binary_search_by_key(&addr, |x| x.start_addr()) { + Ok(x) => Some(x), + // Within the closest region with starting address < addr + Err(x) if (x > 0 && addr <= self.regions[x - 1].last_addr()) => Some(x - 1), + _ => None, + }; + index.map(|x| self.regions[x].as_ref()) + } + + fn iter(&self) -> impl Iterator { + self.regions.iter().map(AsRef::as_ref) + } +} + impl Bytes for T { type E = Error; diff --git a/src/lib.rs b/src/lib.rs index 6f87ce48..d796e9d2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -47,7 +47,8 @@ pub use endian::{Be16, Be32, Be64, BeSize, Le16, Le32, Le64, LeSize}; pub mod guest_memory; pub use guest_memory::{ Error as GuestMemoryError, FileOffset, GuestAddress, GuestAddressSpace, GuestMemory, - GuestMemoryRegion, GuestUsize, MemoryRegionAddress, Result as GuestMemoryResult, + GuestMemoryRegion, GuestRegionCollection, GuestRegionCollectionError as Error, GuestUsize, + MemoryRegionAddress, Result as GuestMemoryResult, }; pub mod io; @@ -66,7 +67,7 @@ mod mmap_windows; pub mod mmap; #[cfg(feature = "backend-mmap")] -pub use mmap::{Error, GuestMemoryMmap, GuestRegionMmap, MmapRegion}; +pub use mmap::{GuestMemoryMmap, GuestRegionMmap, MmapRegion}; #[cfg(all(feature = "backend-mmap", feature = "xen", unix))] pub use mmap::{MmapRange, MmapXenFlags}; diff --git a/src/mmap.rs b/src/mmap.rs index a5c81f24..6f0faf7f 100644 --- a/src/mmap.rs +++ b/src/mmap.rs @@ -18,15 +18,15 @@ use std::io::{Seek, SeekFrom}; use std::ops::Deref; use std::result; use std::sync::atomic::Ordering; -use std::sync::Arc; use crate::address::Address; use crate::bitmap::{Bitmap, BS}; use crate::guest_memory::{ - self, FileOffset, GuestAddress, GuestMemory, GuestMemoryRegion, GuestUsize, MemoryRegionAddress, + self, FileOffset, GuestAddress, GuestMemoryRegion, GuestRegionCollection, GuestUsize, + MemoryRegionAddress, }; use crate::volatile_memory::{VolatileMemory, VolatileSlice}; -use crate::{AtomicAccess, Bytes, ReadVolatile, WriteVolatile}; +use crate::{AtomicAccess, Bytes, Error, ReadVolatile, WriteVolatile}; #[cfg(all(not(feature = "xen"), unix))] pub use crate::mmap_unix::{Error as MmapRegionError, MmapRegion, MmapRegionBuilder}; @@ -49,27 +49,6 @@ impl NewBitmap for () { fn with_len(_len: usize) -> Self {} } -/// Errors that can occur when creating a memory map. -#[derive(Debug, thiserror::Error)] -pub enum Error { - /// Adding the guest base address to the length of the underlying mapping resulted - /// in an overflow. - #[error("Adding the guest base address to the length of the underlying mapping resulted in an overflow")] - InvalidGuestRegion, - /// Error creating a `MmapRegion` object. - #[error("{0}")] - MmapRegion(MmapRegionError), - /// No memory region found. - #[error("No memory region found")] - NoMemoryRegion, - /// Some of the memory regions intersect with each other. - #[error("Some of the memory regions intersect with each other")] - MemoryRegionOverlap, - /// The provided memory regions haven't been sorted. - #[error("The provided memory regions haven't been sorted")] - UnsortedMemoryRegions, -} - // 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`. @@ -366,17 +345,9 @@ impl GuestMemoryRegion for GuestRegionMmap { /// Represents the entire physical memory of the guest by tracking all its memory regions. /// Each region is an instance of `GuestRegionMmap`, being backed by a mapping in the /// virtual address space of the calling process. -#[derive(Clone, Debug, Default)] -pub struct GuestMemoryMmap { - regions: Vec>>, -} +pub type GuestMemoryMmap = GuestRegionCollection>; impl GuestMemoryMmap { - /// Creates an empty `GuestMemoryMmap` instance. - pub fn new() -> Self { - Self::default() - } - /// Creates a container and allocates anonymous memory for guest memory regions. /// /// Valid memory regions are specified as a slice of (Address, Size) tuples sorted by Address. @@ -404,111 +375,6 @@ impl GuestMemoryMmap { } } -impl GuestMemoryMmap { - /// Creates a new `GuestMemoryMmap` from a vector of regions. - /// - /// # Arguments - /// - /// * `regions` - The vector of regions. - /// The regions shouldn't overlap and they should be sorted - /// by the starting address. - pub fn from_regions(mut regions: Vec>) -> result::Result { - Self::from_arc_regions(regions.drain(..).map(Arc::new).collect()) - } - - /// Creates a new `GuestMemoryMmap` from a vector of Arc regions. - /// - /// Similar to the constructor `from_regions()` as it returns a - /// `GuestMemoryMmap`. The need for this constructor is to provide a way for - /// consumer of this API to create a new `GuestMemoryMmap` based on existing - /// regions coming from an existing `GuestMemoryMmap` instance. - /// - /// # Arguments - /// - /// * `regions` - The vector of `Arc` regions. - /// The regions shouldn't overlap and they should be sorted - /// by the starting address. - pub fn from_arc_regions(regions: Vec>>) -> result::Result { - if regions.is_empty() { - return Err(Error::NoMemoryRegion); - } - - for window in regions.windows(2) { - let prev = &window[0]; - let next = &window[1]; - - if prev.start_addr() > next.start_addr() { - return Err(Error::UnsortedMemoryRegions); - } - - if prev.last_addr() >= next.start_addr() { - return Err(Error::MemoryRegionOverlap); - } - } - - Ok(Self { regions }) - } - - /// Insert a region into the `GuestMemoryMmap` object and return a new `GuestMemoryMmap`. - /// - /// # Arguments - /// * `region`: the memory region to insert into the guest memory object. - pub fn insert_region( - &self, - region: Arc>, - ) -> result::Result, Error> { - let mut regions = self.regions.clone(); - regions.push(region); - regions.sort_by_key(|x| x.start_addr()); - - Self::from_arc_regions(regions) - } - - /// Remove a region into the `GuestMemoryMmap` object and return a new `GuestMemoryMmap` - /// on success, together with the removed region. - /// - /// # Arguments - /// * `base`: base address of the region to be removed - /// * `size`: size of the region to be removed - pub fn remove_region( - &self, - base: GuestAddress, - size: GuestUsize, - ) -> result::Result<(GuestMemoryMmap, Arc>), Error> { - if let Ok(region_index) = self.regions.binary_search_by_key(&base, |x| x.start_addr()) { - if self.regions.get(region_index).unwrap().mapping.size() as GuestUsize == size { - let mut regions = self.regions.clone(); - let region = regions.remove(region_index); - return Ok((Self { regions }, region)); - } - } - - Err(Error::InvalidGuestRegion) - } -} - -impl GuestMemory for GuestMemoryMmap { - type R = GuestRegionMmap; - - fn num_regions(&self) -> usize { - self.regions.len() - } - - fn find_region(&self, addr: GuestAddress) -> Option<&GuestRegionMmap> { - let index = match self.regions.binary_search_by_key(&addr, |x| x.start_addr()) { - Ok(x) => Some(x), - // Within the closest region with starting address < addr - Err(x) if (x > 0 && addr <= self.regions[x - 1].last_addr()) => Some(x - 1), - _ => None, - }; - index.map(|x| self.regions[x].as_ref()) - } - - fn iter(&self) -> impl Iterator { - self.regions.iter().map(AsRef::as_ref) - } -} - #[cfg(test)] mod tests { #![allow(clippy::undocumented_unsafe_blocks)] @@ -518,16 +384,17 @@ mod tests { use crate::bitmap::tests::test_guest_memory_and_region; use crate::bitmap::AtomicBitmap; - use crate::GuestAddressSpace; + use crate::{Error, GuestAddressSpace, GuestMemory, GuestMemoryError}; use std::io::Write; use std::mem; + use std::sync::Arc; #[cfg(feature = "rawfd")] use std::{fs::File, path::Path}; use vmm_sys_util::tempfile::TempFile; - type GuestMemoryMmap = super::GuestMemoryMmap<()>; type GuestRegionMmap = super::GuestRegionMmap<()>; + type GuestMemoryMmap = super::GuestRegionCollection; type MmapRegion = super::MmapRegion<()>; #[test] @@ -552,9 +419,8 @@ mod tests { } assert_eq!(guest_mem.last_addr(), last_addr); } - for ((region_addr, region_size), mmap) in expected_regions_summary - .iter() - .zip(guest_mem.regions.iter()) + for ((region_addr, region_size), mmap) in + expected_regions_summary.iter().zip(guest_mem.iter()) { assert_eq!(region_addr, &mmap.guest_base); assert_eq!(region_size, &mmap.mapping.size()); @@ -615,129 +481,66 @@ mod tests { fn test_no_memory_region() { let regions_summary = []; - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap(®ions_summary).err().unwrap() - ), - format!("{:?}", Error::NoMemoryRegion) - ); - - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap_with_files(®ions_summary) - .err() - .unwrap() - ), - format!("{:?}", Error::NoMemoryRegion) - ); - - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap_from_regions(®ions_summary) - .err() - .unwrap() - ), - format!("{:?}", Error::NoMemoryRegion) - ); - - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap_from_arc_regions(®ions_summary) - .err() - .unwrap() - ), - format!("{:?}", Error::NoMemoryRegion) - ); + assert!(matches!( + new_guest_memory_mmap(®ions_summary).unwrap_err(), + Error::NoMemoryRegion + )); + assert!(matches!( + new_guest_memory_mmap_with_files(®ions_summary).unwrap_err(), + Error::NoMemoryRegion + )); + assert!(matches!( + new_guest_memory_mmap_from_regions(®ions_summary).unwrap_err(), + Error::NoMemoryRegion + )); + assert!(matches!( + new_guest_memory_mmap_from_regions(®ions_summary).unwrap_err(), + Error::NoMemoryRegion + )); } #[test] fn test_overlapping_memory_regions() { let regions_summary = [(GuestAddress(0), 100_usize), (GuestAddress(99), 100_usize)]; - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap(®ions_summary).err().unwrap() - ), - format!("{:?}", Error::MemoryRegionOverlap) - ); - - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap_with_files(®ions_summary) - .err() - .unwrap() - ), - format!("{:?}", Error::MemoryRegionOverlap) - ); - - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap_from_regions(®ions_summary) - .err() - .unwrap() - ), - format!("{:?}", Error::MemoryRegionOverlap) - ); - - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap_from_arc_regions(®ions_summary) - .err() - .unwrap() - ), - format!("{:?}", Error::MemoryRegionOverlap) - ); + assert!(matches!( + new_guest_memory_mmap(®ions_summary).unwrap_err(), + Error::MemoryRegionOverlap + )); + assert!(matches!( + new_guest_memory_mmap_with_files(®ions_summary).unwrap_err(), + Error::MemoryRegionOverlap + )); + assert!(matches!( + new_guest_memory_mmap_from_regions(®ions_summary).unwrap_err(), + Error::MemoryRegionOverlap + )); + assert!(matches!( + new_guest_memory_mmap_from_regions(®ions_summary).unwrap_err(), + Error::MemoryRegionOverlap + )); } #[test] fn test_unsorted_memory_regions() { let regions_summary = [(GuestAddress(100), 100_usize), (GuestAddress(0), 100_usize)]; - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap(®ions_summary).err().unwrap() - ), - format!("{:?}", Error::UnsortedMemoryRegions) - ); - - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap_with_files(®ions_summary) - .err() - .unwrap() - ), - format!("{:?}", Error::UnsortedMemoryRegions) - ); - - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap_from_regions(®ions_summary) - .err() - .unwrap() - ), - format!("{:?}", Error::UnsortedMemoryRegions) - ); - - assert_eq!( - format!( - "{:?}", - new_guest_memory_mmap_from_arc_regions(®ions_summary) - .err() - .unwrap() - ), - format!("{:?}", Error::UnsortedMemoryRegions) - ); + assert!(matches!( + new_guest_memory_mmap(®ions_summary).unwrap_err(), + Error::UnsortedMemoryRegions + )); + assert!(matches!( + new_guest_memory_mmap_with_files(®ions_summary).unwrap_err(), + Error::UnsortedMemoryRegions + )); + assert!(matches!( + new_guest_memory_mmap_from_regions(®ions_summary).unwrap_err(), + Error::UnsortedMemoryRegions + )); + assert!(matches!( + new_guest_memory_mmap_from_regions(®ions_summary).unwrap_err(), + Error::UnsortedMemoryRegions + )); } #[test] @@ -745,7 +548,7 @@ mod tests { let regions_summary = [(GuestAddress(0), 100_usize), (GuestAddress(100), 100_usize)]; let guest_mem = GuestMemoryMmap::new(); - assert_eq!(guest_mem.regions.len(), 0); + assert_eq!(guest_mem.num_regions(), 0); check_guest_memory_mmap(new_guest_memory_mmap(®ions_summary), ®ions_summary); @@ -962,18 +765,13 @@ mod tests { for gm in gm_list.iter() { let val1: u64 = 0xaa55_aa55_aa55_aa55; let val2: u64 = 0x55aa_55aa_55aa_55aa; - assert_eq!( - format!("{:?}", gm.write_obj(val1, bad_addr).err().unwrap()), - format!("InvalidGuestAddress({:?})", bad_addr,) - ); - assert_eq!( - format!("{:?}", gm.write_obj(val1, bad_addr2).err().unwrap()), - format!( - "PartialBuffer {{ expected: {:?}, completed: {:?} }}", - mem::size_of::(), - max_addr.checked_offset_from(bad_addr2).unwrap() - ) - ); + assert!(matches!( + gm.write_obj(val1, bad_addr).unwrap_err(), + GuestMemoryError::InvalidGuestAddress(addr) if addr == bad_addr + )); + assert!(matches!( + gm.write_obj(val1, bad_addr2).unwrap_err(), + GuestMemoryError::PartialBuffer { expected, completed} if expected == size_of::() && completed == max_addr.checked_offset_from(bad_addr2).unwrap() as usize)); gm.write_obj(val1, GuestAddress(0x500)).unwrap(); gm.write_obj(val2, GuestAddress(0x1000 + 32)).unwrap(); @@ -1083,8 +881,10 @@ mod tests { .map(|x| (x.0, x.1)) .eq(iterated_regions.iter().copied())); - assert_eq!(gm.regions[0].guest_base, regions[0].0); - assert_eq!(gm.regions[1].guest_base, regions[1].0); + let mmap_regions = gm.iter().collect::>(); + + assert_eq!(mmap_regions[0].guest_base, regions[0].0); + assert_eq!(mmap_regions[1].guest_base, regions[1].0); } #[test] @@ -1112,8 +912,10 @@ mod tests { .map(|x| (x.0, x.1)) .eq(iterated_regions.iter().copied())); - assert_eq!(gm.regions[0].guest_base, regions[0].0); - assert_eq!(gm.regions[1].guest_base, regions[1].0); + let mmap_regions = gm.iter().collect::>(); + + assert_eq!(mmap_regions[0].guest_base, regions[0].0); + assert_eq!(mmap_regions[1].guest_base, regions[1].0); } #[test] @@ -1222,11 +1024,13 @@ mod tests { assert_eq!(mem_orig.num_regions(), 2); assert_eq!(gm.num_regions(), 5); - assert_eq!(gm.regions[0].start_addr(), GuestAddress(0x0000)); - assert_eq!(gm.regions[1].start_addr(), GuestAddress(0x4000)); - assert_eq!(gm.regions[2].start_addr(), GuestAddress(0x8000)); - assert_eq!(gm.regions[3].start_addr(), GuestAddress(0xc000)); - assert_eq!(gm.regions[4].start_addr(), GuestAddress(0x10_0000)); + let regions = gm.iter().collect::>(); + + assert_eq!(regions[0].start_addr(), GuestAddress(0x0000)); + assert_eq!(regions[1].start_addr(), GuestAddress(0x4000)); + assert_eq!(regions[2].start_addr(), GuestAddress(0x8000)); + assert_eq!(regions[3].start_addr(), GuestAddress(0xc000)); + assert_eq!(regions[4].start_addr(), GuestAddress(0x10_0000)); } #[test] @@ -1247,7 +1051,7 @@ mod tests { assert_eq!(mem_orig.num_regions(), 2); assert_eq!(gm.num_regions(), 1); - assert_eq!(gm.regions[0].start_addr(), GuestAddress(0x0000)); + assert_eq!(gm.iter().next().unwrap().start_addr(), GuestAddress(0x0000)); assert_eq!(region.start_addr(), GuestAddress(0x10_0000)); } diff --git a/src/mmap_unix.rs b/src/mmap_unix.rs index 14ceb809..667c7f9b 100644 --- a/src/mmap_unix.rs +++ b/src/mmap_unix.rs @@ -558,7 +558,7 @@ mod tests { prot, flags, ); - assert_eq!(format!("{:?}", r.unwrap_err()), "InvalidOffsetLength"); + assert!(matches!(r.unwrap_err(), Error::InvalidOffsetLength)); // Offset + size is greater than the size of the file (which is 0 at this point). let r = MmapRegion::build( @@ -567,7 +567,7 @@ mod tests { prot, flags, ); - assert_eq!(format!("{:?}", r.unwrap_err()), "MappingPastEof"); + assert!(matches!(r.unwrap_err(), Error::MappingPastEof)); // MAP_FIXED was specified among the flags. let r = MmapRegion::build( @@ -576,7 +576,7 @@ mod tests { prot, flags | libc::MAP_FIXED, ); - assert_eq!(format!("{:?}", r.unwrap_err()), "MapFixed"); + assert!(matches!(r.unwrap_err(), Error::MapFixed)); // Let's resize the file. assert_eq!(unsafe { libc::ftruncate(a.as_raw_fd(), 1024 * 10) }, 0); @@ -621,7 +621,7 @@ mod tests { let flags = libc::MAP_NORESERVE | libc::MAP_PRIVATE; let r = unsafe { MmapRegion::build_raw((addr + 1) as *mut u8, size, prot, flags) }; - assert_eq!(format!("{:?}", r.unwrap_err()), "InvalidPointer"); + assert!(matches!(r.unwrap_err(), Error::InvalidPointer)); let r = unsafe { MmapRegion::build_raw(addr as *mut u8, size, prot, flags).unwrap() }; diff --git a/src/mmap_xen.rs b/src/mmap_xen.rs index b49495a3..b88db594 100644 --- a/src/mmap_xen.rs +++ b/src/mmap_xen.rs @@ -1077,26 +1077,18 @@ mod tests { range.mmap_flags = 16; let r = MmapXen::new(&range); - assert_eq!( - format!("{:?}", r.unwrap_err()), - format!("MmapFlags({})", range.mmap_flags), - ); + assert!(matches!(r.unwrap_err(), Error::MmapFlags(flags) if flags == range.mmap_flags)); range.mmap_flags = MmapXenFlags::FOREIGN.bits() | MmapXenFlags::GRANT.bits(); let r = MmapXen::new(&range); - assert_eq!( - format!("{:?}", r.unwrap_err()), - format!("MmapFlags({:x})", MmapXenFlags::ALL.bits()), + assert!( + matches!(r.unwrap_err(), Error::MmapFlags(flags) if flags == MmapXenFlags::ALL.bits()) ); range.mmap_flags = MmapXenFlags::FOREIGN.bits() | MmapXenFlags::NO_ADVANCE_MAP.bits(); let r = MmapXen::new(&range); - assert_eq!( - format!("{:?}", r.unwrap_err()), - format!( - "MmapFlags({:x})", - MmapXenFlags::NO_ADVANCE_MAP.bits() | MmapXenFlags::FOREIGN.bits(), - ), + assert!( + matches!(r.unwrap_err(), Error::MmapFlags(flags) if flags == MmapXenFlags::NO_ADVANCE_MAP.bits() | MmapXenFlags::FOREIGN.bits()) ); } @@ -1132,17 +1124,17 @@ mod tests { range.file_offset = Some(FileOffset::new(TempFile::new().unwrap().into_file(), 0)); range.prot = None; let r = MmapXenForeign::new(&range); - assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError"); + assert!(matches!(r.unwrap_err(), Error::UnexpectedError)); let mut range = MmapRange::initialized(true); range.flags = None; let r = MmapXenForeign::new(&range); - assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError"); + assert!(matches!(r.unwrap_err(), Error::UnexpectedError)); let mut range = MmapRange::initialized(true); range.file_offset = Some(FileOffset::new(TempFile::new().unwrap().into_file(), 1)); let r = MmapXenForeign::new(&range); - assert_eq!(format!("{:?}", r.unwrap_err()), "InvalidOffsetLength"); + assert!(matches!(r.unwrap_err(), Error::InvalidOffsetLength)); let mut range = MmapRange::initialized(true); range.size = 0; @@ -1164,7 +1156,7 @@ mod tests { let mut range = MmapRange::initialized(true); range.prot = None; let r = MmapXenGrant::new(&range, MmapXenFlags::empty()); - assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError"); + assert!(matches!(r.unwrap_err(), Error::UnexpectedError)); let mut range = MmapRange::initialized(true); range.prot = None; @@ -1174,12 +1166,12 @@ mod tests { let mut range = MmapRange::initialized(true); range.flags = None; let r = MmapXenGrant::new(&range, MmapXenFlags::NO_ADVANCE_MAP); - assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError"); + assert!(matches!(r.unwrap_err(), Error::UnexpectedError)); 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_eq!(format!("{:?}", r.unwrap_err()), "InvalidOffsetLength"); + assert!(matches!(r.unwrap_err(), Error::InvalidOffsetLength)); let mut range = MmapRange::initialized(true); range.size = 0; diff --git a/src/volatile_memory.rs b/src/volatile_memory.rs index 486f5720..bd0b0f59 100644 --- a/src/volatile_memory.rs +++ b/src/volatile_memory.rs @@ -1488,58 +1488,6 @@ mod tests { slice.compute_end_offset(6, 0).unwrap_err(); } - #[test] - fn test_display_error() { - assert_eq!( - format!("{}", Error::OutOfBounds { addr: 0x10 }), - "address 0x10 is out of bounds" - ); - - assert_eq!( - format!( - "{}", - Error::Overflow { - base: 0x0, - offset: 0x10 - } - ), - "address 0x0 offset by 0x10 would overflow" - ); - - assert_eq!( - format!( - "{}", - Error::TooBig { - nelements: 100_000, - size: 1_000_000_000 - } - ), - "100000 elements of size 1000000000 would overflow a usize" - ); - - assert_eq!( - format!( - "{}", - Error::Misaligned { - addr: 0x4, - alignment: 8 - } - ), - "address 0x4 is not aligned to 8" - ); - - assert_eq!( - format!( - "{}", - Error::PartialBuffer { - expected: 100, - completed: 90 - } - ), - "only used 90 bytes in 100 long buffer" - ); - } - #[test] fn misaligned_ref() { let mut a = [0u8; 3];