Rollup merge of rust-lang#128092 - ChrisDenton:wrappers, r=workingjubilee

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.

Author: matthiaskrgr

std/src/sys/pal/windows/c.rs

@@ -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;
@@ -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"))] {
@@ -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
     }

std/src/sys/pal/windows/handle.rs

@@ -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(),
             )
         };
 
@@ -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 {

std/src/sys/pal/windows/pipe.rs

@@ -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
@@ -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 {
@@ -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 {
@@ -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)
+            })
         }
     }
 
@@ -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.
@@ -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.
@@ -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;
             }