Use `HandleOrNull` and `HandleOrInvalid` in the Windows FFI bindings.

Use the new `HandleOrNull` and `HandleOrInvalid` types that were introduced
as part of [I/O safety] in a few functions in the Windows FFI bindings.

This factors out an `unsafe` block and two `unsafe` function calls in the
Windows implementation code.

And, it helps test `HandleOrNull` and `HandleOrInvalid`, which indeed turned
up a bug: `OwnedHandle` also needs to be `#[repr(transparent)]`, as it's
used inside of `HandleOrNull` and `HandleOrInvalid` which are also
`#[repr(transparent)]`.

[I/O safety]: https://github.com/rust-lang/rust/issues/87074

diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs
index d378d591ba3a9f4c5508ba9c2c6c931ef02d25e8..14b94d8dcdf92e889d2921362eee551949229365 100644 (file)
--- a/library/std/src/os/windows/io/handle.rs
+++ b/library/std/src/os/windows/io/handle.rs
@@ -59,6 +59,7 @@ pub struct BorrowedHandle<'handle> {
 /// [`RegCloseKey`]: https://docs.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regclosekey
 ///
 /// [here]: https://devblogs.microsoft.com/oldnewthing/20040302-00/?p=40443
+#[repr(transparent)]
 #[unstable(feature = "io_safety", issue = "87074")]
 pub struct OwnedHandle {
     handle: RawHandle,

diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs
index 2affd7e75b03082b3f49d2a2f2e4dcb1c9007932..9b61b2476d5bbdd55c811b00d9493b098215cb5b 100644 (file)
--- a/library/std/src/sys/windows/c.rs
+++ b/library/std/src/sys/windows/c.rs
@@ -6,6 +6,7 @@
 
 use crate::mem;
 use crate::os::raw::{c_char, c_int, c_long, c_longlong, c_uint, c_ulong, c_ushort};
+use crate::os::windows::io::{BorrowedHandle, HandleOrInvalid, HandleOrNull};
 use crate::ptr;
 use core::ffi::NonZero_c_ulong;
 
@@ -886,7 +887,7 @@ pub fn CreateThread(
         lpParameter: LPVOID,
         dwCreationFlags: DWORD,
         lpThreadId: LPDWORD,
-    ) -> HANDLE;
+    ) -> HandleOrNull;
     pub fn WaitForSingleObject(hHandle: HANDLE, dwMilliseconds: DWORD) -> DWORD;
     pub fn SwitchToThread() -> BOOL;
     pub fn Sleep(dwMilliseconds: DWORD);
@@ -950,14 +951,14 @@ pub fn DuplicateHandle(
         dwOptions: DWORD,
     ) -> BOOL;
     pub fn ReadFile(
-        hFile: HANDLE,
+        hFile: BorrowedHandle<'_>,
         lpBuffer: LPVOID,
         nNumberOfBytesToRead: DWORD,
         lpNumberOfBytesRead: LPDWORD,
         lpOverlapped: LPOVERLAPPED,
     ) -> BOOL;
     pub fn WriteFile(
-        hFile: HANDLE,
+        hFile: BorrowedHandle<'_>,
         lpBuffer: LPVOID,
         nNumberOfBytesToWrite: DWORD,
         lpNumberOfBytesWritten: LPDWORD,
@@ -981,7 +982,7 @@ pub fn CreateFileW(
         dwCreationDisposition: DWORD,
         dwFlagsAndAttributes: DWORD,
         hTemplateFile: HANDLE,
-    ) -> HANDLE;
+    ) -> HandleOrInvalid;
 
     pub fn FindFirstFileW(fileName: LPCWSTR, findFileData: LPWIN32_FIND_DATAW) -> HANDLE;
     pub fn FindNextFileW(findFile: HANDLE, findFileData: LPWIN32_FIND_DATAW) -> BOOL;

diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs
index cb83ee2469a1c685f204f704550ca8be42d398de..d6c40a15329a96a9091aa9c6936d1fed7e1ad79c 100644 (file)
--- a/library/std/src/sys/windows/fs.rs
+++ b/library/std/src/sys/windows/fs.rs
@@ -1,5 +1,6 @@
 use crate::os::windows::prelude::*;
 
+use crate::convert::TryInto;
 use crate::ffi::OsString;
 use crate::fmt;
 use crate::io::{self, Error, IoSlice, IoSliceMut, ReadBuf, SeekFrom};
@@ -294,10 +295,10 @@ pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
                 ptr::null_mut(),
             )
         };
-        if handle == c::INVALID_HANDLE_VALUE {
-            Err(Error::last_os_error())
+        if let Ok(handle) = handle.try_into() {
+            Ok(File { handle: Handle::from_inner(handle) })
         } else {
-            unsafe { Ok(File { handle: Handle::from_raw_handle(handle) }) }
+            Err(Error::last_os_error())
         }
     }
 

diff --git a/library/std/src/sys/windows/handle.rs b/library/std/src/sys/windows/handle.rs
index daab39bb00cbcd9a0c556d935bbfc66b7b697953..e5c9567957bc1db5c5e52c9eb0bd5fde8de807aa 100644 (file)
--- a/library/std/src/sys/windows/handle.rs
+++ b/library/std/src/sys/windows/handle.rs
@@ -78,7 +78,7 @@ pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
         let len = cmp::min(buf.len(), <c::DWORD>::MAX as usize) as c::DWORD;
         let res = cvt(unsafe {
             c::ReadFile(
-                self.as_raw_handle(),
+                self.as_handle(),
                 buf.as_mut_ptr() as c::LPVOID,
                 len,
                 &mut read,
@@ -116,7 +116,7 @@ pub fn read_at(&self, buf: &mut [u8], offset: u64) -> io::Result<usize> {
             overlapped.Offset = offset as u32;
             overlapped.OffsetHigh = (offset >> 32) as u32;
             cvt(c::ReadFile(
-                self.as_raw_handle(),
+                self.as_handle(),
                 buf.as_mut_ptr() as c::LPVOID,
                 len,
                 &mut read,
@@ -135,7 +135,7 @@ pub fn read_buf(&self, buf: &mut ReadBuf<'_>) -> io::Result<()> {
         let len = cmp::min(buf.remaining(), <c::DWORD>::MAX as usize) as c::DWORD;
         let res = cvt(unsafe {
             c::ReadFile(
-                self.as_raw_handle(),
+                self.as_handle(),
                 buf.unfilled_mut().as_mut_ptr() as c::LPVOID,
                 len,
                 &mut read,
@@ -171,7 +171,7 @@ pub unsafe fn read_overlapped(
         let len = cmp::min(buf.len(), <c::DWORD>::MAX as usize) as c::DWORD;
         let mut amt = 0;
         let res = cvt(c::ReadFile(
-            self.as_raw_handle(),
+            self.as_handle(),
             buf.as_ptr() as c::LPVOID,
             len,
             &mut amt,
@@ -225,7 +225,7 @@ pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
         let len = cmp::min(buf.len(), <c::DWORD>::MAX as usize) as c::DWORD;
         cvt(unsafe {
             c::WriteFile(
-                self.as_raw_handle(),
+                self.as_handle(),
                 buf.as_ptr() as c::LPVOID,
                 len,
                 &mut amt,
@@ -252,7 +252,7 @@ pub fn write_at(&self, buf: &[u8], offset: u64) -> io::Result<usize> {
             overlapped.Offset = offset as u32;
             overlapped.OffsetHigh = (offset >> 32) as u32;
             cvt(c::WriteFile(
-                self.as_raw_handle(),
+                self.as_handle(),
                 buf.as_ptr() as c::LPVOID,
                 len,
                 &mut written,

diff --git a/library/std/src/sys/windows/thread.rs b/library/std/src/sys/windows/thread.rs
index e4bba9255d23e774849196a88f09ee882638be2c..bd304dc57371dda02bfc4250f71f279e5f7ad318 100644 (file)
--- a/library/std/src/sys/windows/thread.rs
+++ b/library/std/src/sys/windows/thread.rs
@@ -1,11 +1,13 @@
+use crate::convert::TryInto;
 use crate::ffi::CStr;
 use crate::io;
 use crate::num::NonZeroUsize;
-use crate::os::windows::io::{AsRawHandle, FromRawHandle};
+use crate::os::windows::io::AsRawHandle;
 use crate::ptr;
 use crate::sys::c;
 use crate::sys::handle::Handle;
 use crate::sys::stack_overflow;
+use crate::sys_common::FromInner;
 use crate::time::Duration;
 
 use libc::c_void;
@@ -40,13 +42,13 @@ pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
             ptr::null_mut(),
         );
 
-        return if ret as usize == 0 {
+        return if let Ok(handle) = ret.try_into() {
+            Ok(Thread { handle: Handle::from_inner(handle) })
+        } else {
             // The thread failed to start and as a result p was not consumed. Therefore, it is
             // safe to reconstruct the box so that it gets deallocated.
             drop(Box::from_raw(p));
             Err(io::Error::last_os_error())
-        } else {
-            Ok(Thread { handle: Handle::from_raw_handle(ret) })
         };
 
         extern "system" fn thread_start(main: *mut c_void) -> c::DWORD {