Skip to content

Commit

Permalink
Rollup merge of rust-lang#94572 - sunfishcode:sunfishcode/handle-or, …
Browse files Browse the repository at this point in the history
…r=joshtriplett

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`, and indeed, it
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)]`.

r? `@joshtriplett`

[I/O safety]: rust-lang#87074
  • Loading branch information
Dylan-DPC authored Mar 4, 2022
2 parents 0f7a27c + 3560649 commit a4d1149
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 17 deletions.
1 change: 1 addition & 0 deletions library/std/src/os/windows/io/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 5 additions & 4 deletions library/std/src/sys/windows/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -886,7 +887,7 @@ extern "system" {
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);
Expand Down Expand Up @@ -950,14 +951,14 @@ extern "system" {
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,
Expand All @@ -981,7 +982,7 @@ extern "system" {
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;
Expand Down
7 changes: 4 additions & 3 deletions library/std/src/sys/windows/fs.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -294,10 +295,10 @@ impl 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())
}
}

Expand Down
12 changes: 6 additions & 6 deletions library/std/src/sys/windows/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl Handle {
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,
Expand Down Expand Up @@ -116,7 +116,7 @@ impl Handle {
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,
Expand All @@ -135,7 +135,7 @@ impl Handle {
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,
Expand Down Expand Up @@ -171,7 +171,7 @@ impl Handle {
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,
Expand Down Expand Up @@ -225,7 +225,7 @@ impl Handle {
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,
Expand All @@ -252,7 +252,7 @@ impl Handle {
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,
Expand Down
10 changes: 6 additions & 4 deletions library/std/src/sys/windows/thread.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -40,13 +42,13 @@ impl 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 {
Expand Down

0 comments on commit a4d1149

Please sign in to comment.