Skip to content

Misc cleanups: move mmap files and replace cfg unix/windows with target_family #314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ bitflags = { version = "2.4.0", optional = true }
thiserror = "1.0.40"
vmm-sys-util = { version = "0.12.1", optional = true }

[target.'cfg(windows)'.dependencies.winapi]
[target.'cfg(target_family = "windows")'.dependencies.winapi]
version = "0.3"
features = ["errhandlingapi", "sysinfoapi"]

Expand Down
2 changes: 1 addition & 1 deletion coverage_config_aarch64.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"coverage_score": 85.2,
"exclude_path": "mmap_windows.rs",
"exclude_path": "mmap/windows.rs",
"crate_features": "backend-mmap,backend-atomic,backend-bitmap"
}
4 changes: 2 additions & 2 deletions src/bitmap/backend/atomic_bitmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,11 @@ impl Default for AtomicBitmap {
#[cfg(feature = "backend-mmap")]
impl NewBitmap for AtomicBitmap {
fn with_len(len: usize) -> Self {
#[cfg(unix)]
#[cfg(target_family = "unix")]
// SAFETY: There's no unsafe potential in calling this function.
let page_size = unsafe { libc::sysconf(libc::_SC_PAGE_SIZE) };

#[cfg(windows)]
#[cfg(target_family = "windows")]
let page_size = {
use winapi::um::sysinfoapi::{GetSystemInfo, LPSYSTEM_INFO, SYSTEM_INFO};
let mut sysinfo = std::mem::MaybeUninit::zeroed();
Expand Down
2 changes: 1 addition & 1 deletion src/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ pub trait Bytes<A> {
/// # let gm = GuestMemoryMmap::<()>::from_ranges(&vec![(start_addr, 0x400)])
/// # .expect("Could not create guest memory");
/// # let addr = GuestAddress(0x1010);
/// # let mut file = if cfg!(unix) {
/// # let mut file = if cfg!(target_family = "unix") {
/// let mut file = File::open(Path::new("/dev/urandom")).expect("Could not open /dev/urandom");
/// # file
/// # } else {
Expand Down
11 changes: 1 addition & 10 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,12 @@ pub use guest_memory::{
pub mod io;
pub use io::{ReadVolatile, WriteVolatile};

#[cfg(all(feature = "backend-mmap", not(feature = "xen"), unix))]
mod mmap_unix;

#[cfg(all(feature = "backend-mmap", feature = "xen", unix))]
mod mmap_xen;

#[cfg(all(feature = "backend-mmap", windows))]
mod mmap_windows;

#[cfg(feature = "backend-mmap")]
pub mod mmap;

#[cfg(feature = "backend-mmap")]
pub use mmap::{Error, GuestMemoryMmap, GuestRegionMmap, MmapRegion};
#[cfg(all(feature = "backend-mmap", feature = "xen", unix))]
#[cfg(all(feature = "backend-mmap", feature = "xen", target_family = "unix"))]
pub use mmap::{MmapRange, MmapXenFlags};

pub mod volatile_memory;
Expand Down
35 changes: 22 additions & 13 deletions src/mmap.rs → src/mmap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
//! This implementation is mmap-ing the memory of the guest into the current process.

use std::borrow::Borrow;
#[cfg(unix)]
#[cfg(target_family = "unix")]
use std::io::{Seek, SeekFrom};
use std::ops::Deref;
use std::result;
Expand All @@ -28,16 +28,25 @@ use crate::guest_memory::{
use crate::volatile_memory::{VolatileMemory, VolatileSlice};
use crate::{AtomicAccess, Bytes, ReadVolatile, WriteVolatile};

#[cfg(all(not(feature = "xen"), unix))]
pub use crate::mmap_unix::{Error as MmapRegionError, MmapRegion, MmapRegionBuilder};
#[cfg(all(not(feature = "xen"), target_family = "unix"))]
mod unix;

#[cfg(all(feature = "xen", unix))]
pub use crate::mmap_xen::{Error as MmapRegionError, MmapRange, MmapRegion, MmapXenFlags};
#[cfg(all(feature = "xen", target_family = "unix"))]
pub(crate) mod xen;

#[cfg(windows)]
pub use crate::mmap_windows::MmapRegion;
#[cfg(windows)]
#[cfg(target_family = "windows")]
mod windows;

#[cfg(all(not(feature = "xen"), target_family = "unix"))]
pub use unix::{Error as MmapRegionError, MmapRegion, MmapRegionBuilder};

#[cfg(all(feature = "xen", target_family = "unix"))]
pub use xen::{Error as MmapRegionError, MmapRange, MmapRegion, MmapXenFlags};

#[cfg(target_family = "windows")]
pub use std::io::Error as MmapRegionError;
#[cfg(target_family = "windows")]
pub use windows::MmapRegion;

/// A `Bitmap` that can be created starting from an initial size.
pub trait NewBitmap: Bitmap + Default {
Expand Down Expand Up @@ -71,7 +80,7 @@ pub enum Error {
}

// TODO: use this for Windows as well after we redefine the Error type there.
#[cfg(unix)]
#[cfg(target_family = "unix")]
/// Checks if a mapping of `size` bytes fits at the provided `file_offset`.
///
/// For a borrowed `FileOffset` and size, this function checks whether the mapping does not
Expand Down Expand Up @@ -1033,7 +1042,7 @@ mod tests {
let gm_list = [gm, gm_backed_by_file];
for gm in gm_list.iter() {
let addr = GuestAddress(0x1010);
let mut file = if cfg!(unix) {
let mut file = if cfg!(target_family = "unix") {
File::open(Path::new("/dev/zero")).unwrap()
} else {
File::open(Path::new("c:\\Windows\\system32\\ntoskrnl.exe")).unwrap()
Expand All @@ -1042,7 +1051,7 @@ mod tests {
gm.read_exact_volatile_from(addr, &mut file, mem::size_of::<u32>())
.unwrap();
let value: u32 = gm.read_obj(addr).unwrap();
if cfg!(unix) {
if cfg!(target_family = "unix") {
assert_eq!(value, 0);
} else {
assert_eq!(value, 0x0090_5a4d);
Expand All @@ -1051,7 +1060,7 @@ mod tests {
let mut sink = vec![0; mem::size_of::<u32>()];
gm.write_all_volatile_to(addr, &mut sink.as_mut_slice(), mem::size_of::<u32>())
.unwrap();
if cfg!(unix) {
if cfg!(target_family = "unix") {
assert_eq!(sink, vec![0; mem::size_of::<u32>()]);
} else {
assert_eq!(sink, vec![0x4d, 0x5a, 0x90, 0x00]);
Expand Down Expand Up @@ -1170,7 +1179,7 @@ mod tests {
// used for the backing file. Refer to Microsoft docs here:
// https://docs.microsoft.com/en-us/windows/desktop/api/memoryapi/nf-memoryapi-mapviewoffile
#[test]
#[cfg(unix)]
#[cfg(target_family = "unix")]
fn test_retrieve_offset_from_fd_backing_memory_region() {
let f = TempFile::new().unwrap().into_file();
f.set_len(0x1400).unwrap();
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion src/mmap_windows.rs → src/mmap/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ mod tests {

use crate::bitmap::AtomicBitmap;
use crate::guest_memory::FileOffset;
use crate::mmap_windows::INVALID_HANDLE_VALUE;
use crate::mmap::windows::INVALID_HANDLE_VALUE;

type MmapRegion = super::MmapRegion<()>;

Expand Down
File renamed without changes.
16 changes: 8 additions & 8 deletions src/volatile_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ use crate::atomic_integer::AtomicInteger;
use crate::bitmap::{Bitmap, BitmapSlice, BS};
use crate::{AtomicAccess, ByteValued, Bytes};

#[cfg(all(feature = "backend-mmap", feature = "xen", unix))]
use crate::mmap_xen::{MmapXen as MmapInfo, MmapXenSlice};
#[cfg(all(feature = "backend-mmap", feature = "xen", target_family = "unix"))]
use crate::mmap::xen::{MmapXen as MmapInfo, MmapXenSlice};

#[cfg(not(feature = "xen"))]
type MmapInfo = std::marker::PhantomData<()>;
Expand Down Expand Up @@ -322,15 +322,15 @@ pub struct PtrGuard {

// This isn't used anymore, but it protects the slice from getting unmapped while in use.
// Once this goes out of scope, the memory is unmapped automatically.
#[cfg(all(feature = "xen", unix))]
#[cfg(all(feature = "xen", target_family = "unix"))]
_slice: MmapXenSlice,
}

#[allow(clippy::len_without_is_empty)]
impl PtrGuard {
#[allow(unused_variables)]
fn new(mmap: Option<&MmapInfo>, addr: *mut u8, write: bool, len: usize) -> Self {
#[cfg(all(feature = "xen", unix))]
#[cfg(all(feature = "xen", target_family = "unix"))]
let (addr, _slice) = {
let prot = if write {
libc::PROT_WRITE
Expand All @@ -345,7 +345,7 @@ impl PtrGuard {
addr,
len,

#[cfg(all(feature = "xen", unix))]
#[cfg(all(feature = "xen", target_family = "unix"))]
_slice,
}
}
Expand Down Expand Up @@ -1952,7 +1952,7 @@ mod tests {
let a = VolatileSlice::from(backing.as_mut_slice());
let s = a.as_volatile_slice();
assert!(s.write_obj(!0u32, 1).is_ok());
let mut file = if cfg!(unix) {
let mut file = if cfg!(target_family = "unix") {
File::open(Path::new("/dev/zero")).unwrap()
} else {
File::open(Path::new("c:\\Windows\\system32\\ntoskrnl.exe")).unwrap()
Expand All @@ -1968,7 +1968,7 @@ mod tests {
.is_err());

let value = s.read_obj::<u32>(1).unwrap();
if cfg!(unix) {
if cfg!(target_family = "unix") {
assert_eq!(value, 0);
} else {
assert_eq!(value, 0x0090_5a4d);
Expand All @@ -1980,7 +1980,7 @@ mod tests {
.write_all_volatile(&s.get_slice(1, size_of::<u32>()).unwrap())
.is_ok());

if cfg!(unix) {
if cfg!(target_family = "unix") {
assert_eq!(sink, vec![0; size_of::<u32>()]);
} else {
assert_eq!(sink, vec![0x4d, 0x5a, 0x90, 0x00]);
Expand Down