Skip to content

Commit

Permalink
Rollup merge of #128092 - ChrisDenton:wrappers, r=workingjubilee
Browse files Browse the repository at this point in the history 
Remove wrapper functions from c.rs

I'd like for the windows `c.rs` just to contain the basic platform definitions and not anything higher level unless absolutely necessary. So this removes some wrapper functions that weren't really necessary in any case. The functions are only used in a few places which themselves are relatively thin wrappers. The "interesting" bit is that we had an `AlertableIoFn` that abstracted over `ReadFileEx` and `WriteFileEx`. I've replaced this with a closure.

Also I removed an `#[allow(unsafe_op_in_unsafe_fn)]` while I was moving things around.
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Jul 24, 2024
2 parents e56e886 + 8c3ce60 commit ee77dda
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 126 deletions.
108 changes: 12 additions & 96 deletions library/std/src/sys/pal/windows/c.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
use crate::ffi::CStr;
use crate::mem;
use crate::os::raw::{c_uint, c_ulong, c_ushort, c_void};
use crate::os::windows::io::{AsRawHandle, BorrowedHandle};
use crate::ptr;

pub(super) mod windows_targets;
Expand Down Expand Up @@ -114,89 +113,6 @@ if #[cfg(not(target_vendor = "uwp"))] {
}
}

pub unsafe extern "system" fn WriteFileEx(
hFile: BorrowedHandle<'_>,
lpBuffer: *mut ::core::ffi::c_void,
nNumberOfBytesToWrite: u32,
lpOverlapped: *mut OVERLAPPED,
lpCompletionRoutine: LPOVERLAPPED_COMPLETION_ROUTINE,
) -> BOOL {
windows_sys::WriteFileEx(
hFile.as_raw_handle(),
lpBuffer.cast::<u8>(),
nNumberOfBytesToWrite,
lpOverlapped,
lpCompletionRoutine,
)
}

pub unsafe extern "system" fn ReadFileEx(
hFile: BorrowedHandle<'_>,
lpBuffer: *mut ::core::ffi::c_void,
nNumberOfBytesToRead: u32,
lpOverlapped: *mut OVERLAPPED,
lpCompletionRoutine: LPOVERLAPPED_COMPLETION_ROUTINE,
) -> BOOL {
windows_sys::ReadFileEx(
hFile.as_raw_handle(),
lpBuffer.cast::<u8>(),
nNumberOfBytesToRead,
lpOverlapped,
lpCompletionRoutine,
)
}

cfg_if::cfg_if! {
if #[cfg(not(target_vendor = "uwp"))] {
pub unsafe fn NtReadFile(
filehandle: BorrowedHandle<'_>,
event: HANDLE,
apcroutine: PIO_APC_ROUTINE,
apccontext: *mut c_void,
iostatusblock: &mut IO_STATUS_BLOCK,
buffer: *mut crate::mem::MaybeUninit<u8>,
length: u32,
byteoffset: Option<&i64>,
key: Option<&u32>,
) -> NTSTATUS {
windows_sys::NtReadFile(
filehandle.as_raw_handle(),
event,
apcroutine,
apccontext,
iostatusblock,
buffer.cast::<c_void>(),
length,
byteoffset.map(|o| o as *const i64).unwrap_or(ptr::null()),
key.map(|k| k as *const u32).unwrap_or(ptr::null()),
)
}
pub unsafe fn NtWriteFile(
filehandle: BorrowedHandle<'_>,
event: HANDLE,
apcroutine: PIO_APC_ROUTINE,
apccontext: *mut c_void,
iostatusblock: &mut IO_STATUS_BLOCK,
buffer: *const u8,
length: u32,
byteoffset: Option<&i64>,
key: Option<&u32>,
) -> NTSTATUS {
windows_sys::NtWriteFile(
filehandle.as_raw_handle(),
event,
apcroutine,
apccontext,
iostatusblock,
buffer.cast::<c_void>(),
length,
byteoffset.map(|o| o as *const i64).unwrap_or(ptr::null()),
key.map(|k| k as *const u32).unwrap_or(ptr::null()),
)
}
}
}

// Use raw-dylib to import ProcessPrng as we can't rely on there being an import library.
cfg_if::cfg_if! {
if #[cfg(not(target_vendor = "win7"))] {
Expand Down Expand Up @@ -331,29 +247,29 @@ compat_fn_with_fallback! {
}
#[cfg(target_vendor = "uwp")]
pub fn NtReadFile(
filehandle: BorrowedHandle<'_>,
filehandle: HANDLE,
event: HANDLE,
apcroutine: PIO_APC_ROUTINE,
apccontext: *mut c_void,
iostatusblock: &mut IO_STATUS_BLOCK,
buffer: *mut crate::mem::MaybeUninit<u8>,
apccontext: *const core::ffi::c_void,
iostatusblock: *mut IO_STATUS_BLOCK,
buffer: *mut core::ffi::c_void,
length: u32,
byteoffset: Option<&i64>,
key: Option<&u32>
byteoffset: *const i64,
key: *const u32
) -> NTSTATUS {
STATUS_NOT_IMPLEMENTED
}
#[cfg(target_vendor = "uwp")]
pub fn NtWriteFile(
filehandle: BorrowedHandle<'_>,
filehandle: HANDLE,
event: HANDLE,
apcroutine: PIO_APC_ROUTINE,
apccontext: *mut c_void,
iostatusblock: &mut IO_STATUS_BLOCK,
buffer: *const u8,
apccontext: *const core::ffi::c_void,
iostatusblock: *mut IO_STATUS_BLOCK,
buffer: *const core::ffi::c_void,
length: u32,
byteoffset: Option<&i64>,
key: Option<&u32>
byteoffset: *const i64,
key: *const u32
) -> NTSTATUS {
STATUS_NOT_IMPLEMENTED
}
Expand Down
16 changes: 8 additions & 8 deletions library/std/src/sys/pal/windows/handle.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,15 +250,15 @@ impl Handle {
// the provided `len`.
let status = unsafe {
c::NtReadFile(
self.as_handle(),
self.as_raw_handle(),
ptr::null_mut(),
None,
ptr::null_mut(),
&mut io_status,
buf,
buf.cast::<core::ffi::c_void>(),
len,
offset.map(|n| n as _).as_ref(),
None,
offset.as_ref().map(|n| ptr::from_ref(n).cast::<i64>()).unwrap_or(ptr::null()),
ptr::null(),
)
};

Expand Down Expand Up @@ -300,15 +300,15 @@ impl Handle {
let len = cmp::min(buf.len(), u32::MAX as usize) as u32;
let status = unsafe {
c::NtWriteFile(
self.as_handle(),
self.as_raw_handle(),
ptr::null_mut(),
None,
ptr::null_mut(),
&mut io_status,
buf.as_ptr(),
buf.as_ptr().cast::<core::ffi::c_void>(),
len,
offset.map(|n| n as _).as_ref(),
None,
offset.as_ref().map(|n| ptr::from_ref(n).cast::<i64>()).unwrap_or(ptr::null()),
ptr::null(),
)
};
let status = if status == c::STATUS_PENDING {
Expand Down
45 changes: 23 additions & 22 deletions library/std/src/sys/pal/windows/pipe.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,15 +238,6 @@ fn random_number() -> usize {
}
}

// Abstracts over `ReadFileEx` and `WriteFileEx`
type AlertableIoFn = unsafe extern "system" fn(
BorrowedHandle<'_>,
*mut core::ffi::c_void,
u32,
*mut c::OVERLAPPED,
c::LPOVERLAPPED_COMPLETION_ROUTINE,
) -> c::BOOL;

impl AnonPipe {
pub fn handle(&self) -> &Handle {
&self.inner
Expand All @@ -262,7 +253,10 @@ impl AnonPipe {
pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
let result = unsafe {
let len = crate::cmp::min(buf.len(), u32::MAX as usize) as u32;
self.alertable_io_internal(c::ReadFileEx, buf.as_mut_ptr() as _, len)
let ptr = buf.as_mut_ptr();
self.alertable_io_internal(|overlapped, callback| {
c::ReadFileEx(self.inner.as_raw_handle(), ptr, len, overlapped, callback)
})
};

match result {
Expand All @@ -278,7 +272,10 @@ impl AnonPipe {
pub fn read_buf(&self, mut buf: BorrowedCursor<'_>) -> io::Result<()> {
let result = unsafe {
let len = crate::cmp::min(buf.capacity(), u32::MAX as usize) as u32;
self.alertable_io_internal(c::ReadFileEx, buf.as_mut().as_mut_ptr() as _, len)
let ptr = buf.as_mut().as_mut_ptr().cast::<u8>();
self.alertable_io_internal(|overlapped, callback| {
c::ReadFileEx(self.inner.as_raw_handle(), ptr, len, overlapped, callback)
})
};

match result {
Expand Down Expand Up @@ -313,7 +310,9 @@ impl AnonPipe {
pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
unsafe {
let len = crate::cmp::min(buf.len(), u32::MAX as usize) as u32;
self.alertable_io_internal(c::WriteFileEx, buf.as_ptr() as _, len)
self.alertable_io_internal(|overlapped, callback| {
c::WriteFileEx(self.inner.as_raw_handle(), buf.as_ptr(), len, overlapped, callback)
})
}
}

Expand Down Expand Up @@ -341,12 +340,9 @@ impl AnonPipe {
/// [`ReadFileEx`]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfileex
/// [`WriteFileEx`]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-writefileex
/// [Asynchronous Procedure Call]: https://docs.microsoft.com/en-us/windows/win32/sync/asynchronous-procedure-calls
#[allow(unsafe_op_in_unsafe_fn)]
unsafe fn alertable_io_internal(
&self,
io: AlertableIoFn,
buf: *mut core::ffi::c_void,
len: u32,
io: impl FnOnce(&mut c::OVERLAPPED, c::LPOVERLAPPED_COMPLETION_ROUTINE) -> c::BOOL,
) -> io::Result<usize> {
// Use "alertable I/O" to synchronize the pipe I/O.
// This has four steps.
Expand Down Expand Up @@ -384,20 +380,25 @@ impl AnonPipe {
lpOverlapped: *mut c::OVERLAPPED,
) {
// Set `async_result` using a pointer smuggled through `hEvent`.
let result =
AsyncResult { error: dwErrorCode, transferred: dwNumberOfBytesTransferred };
*(*lpOverlapped).hEvent.cast::<Option<AsyncResult>>() = Some(result);
// SAFETY:
// At this point, the OVERLAPPED struct will have been written to by the OS,
// except for our `hEvent` field which we set to a valid AsyncResult pointer (see below)
unsafe {
let result =
AsyncResult { error: dwErrorCode, transferred: dwNumberOfBytesTransferred };
*(*lpOverlapped).hEvent.cast::<Option<AsyncResult>>() = Some(result);
}
}

// STEP 1: Start the I/O operation.
let mut overlapped: c::OVERLAPPED = crate::mem::zeroed();
let mut overlapped: c::OVERLAPPED = unsafe { crate::mem::zeroed() };
// `hEvent` is unused by `ReadFileEx` and `WriteFileEx`.
// Therefore the documentation suggests using it to smuggle a pointer to the callback.
overlapped.hEvent = core::ptr::addr_of_mut!(async_result) as *mut _;

// Asynchronous read of the pipe.
// If successful, `callback` will be called once it completes.
let result = io(self.inner.as_handle(), buf, len, &mut overlapped, Some(callback));
let result = io(&mut overlapped, Some(callback));
if result == c::FALSE {
// We can return here because the call failed.
// After this we must not return until the I/O completes.
Expand All @@ -408,7 +409,7 @@ impl AnonPipe {
let result = loop {
// STEP 2: Enter an alertable state.
// The second parameter of `SleepEx` is used to make this sleep alertable.
c::SleepEx(c::INFINITE, c::TRUE);
unsafe { c::SleepEx(c::INFINITE, c::TRUE) };
if let Some(result) = async_result {
break result;
}
Expand Down

0 comments on commit ee77dda

Please sign in to comment.