Skip to content
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

Switch to custom bindings for the windows::core internals #1993

Merged
merged 1 commit into from Aug 30, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/build.yml
Expand Up @@ -33,7 +33,7 @@ jobs:
runs-on: windows-2019
strategy:
matrix:
generator: [bindings, windows, sys, yml]
generator: [windows, sys, yml]
steps:
- name: Checkout
uses: actions/checkout@v2
Expand Down Expand Up @@ -184,7 +184,6 @@ jobs:
cargo clippy -p test_win32_arrays &&
cargo clippy -p test_window_long &&
cargo clippy -p test_winrt &&
cargo clippy -p tool_bindings &&
cargo clippy -p tool_gnu &&
cargo clippy -p tool_gnullvm &&
cargo clippy -p tool_ilrs &&
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/test.yml
Expand Up @@ -128,8 +128,8 @@ jobs:
cargo test --target ${{ matrix.target }} -p test_component &&
cargo test --target ${{ matrix.target }} -p test_component_client &&
cargo test --target ${{ matrix.target }} -p test_const_fields &&
cargo test --target ${{ matrix.target }} -p test_core &&
cargo clean &&
cargo test --target ${{ matrix.target }} -p test_core &&
cargo test --target ${{ matrix.target }} -p test_data_object &&
cargo test --target ${{ matrix.target }} -p test_debug &&
cargo test --target ${{ matrix.target }} -p test_deprecated &&
Expand Down Expand Up @@ -168,7 +168,6 @@ jobs:
cargo test --target ${{ matrix.target }} -p test_win32_arrays &&
cargo test --target ${{ matrix.target }} -p test_window_long &&
cargo test --target ${{ matrix.target }} -p test_winrt &&
cargo test --target ${{ matrix.target }} -p tool_bindings &&
cargo test --target ${{ matrix.target }} -p tool_gnu &&
cargo test --target ${{ matrix.target }} -p tool_gnullvm &&
cargo test --target ${{ matrix.target }} -p tool_ilrs &&
Expand Down
6 changes: 3 additions & 3 deletions crates/libs/windows/src/core/agile_reference.rs
Expand Up @@ -10,8 +10,8 @@ pub struct AgileReference<T>(IAgileReference, PhantomData<T>);
impl<T: Interface> AgileReference<T> {
/// Creates an agile reference to the object.
pub fn new(object: &T) -> Result<Self> {
let unknown: &IUnknown = unsafe { std::mem::transmute(object) };
unsafe { RoGetAgileReference(AGILEREFERENCE_DEFAULT, &T::IID, unknown).map(|reference| Self(reference, Default::default())) }
let mut reference = std::mem::MaybeUninit::zeroed();
unsafe { RoGetAgileReference(AGILEREFERENCE_DEFAULT, &T::IID, object.as_raw(), reference.as_mut_ptr()).from_abi(reference).map(|reference| Self(reference, Default::default())) }
}

/// Retrieves a proxy to the target of the `AgileReference` object that may safely be used within any thread context in which get is called.
Expand All @@ -25,6 +25,6 @@ unsafe impl<T: Interface> Sync for AgileReference<T> {}

impl<T> std::fmt::Debug for AgileReference<T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "AgileReference({:?})", <&IAgileReference as Into<&IUnknown>>::into(&self.0))
write!(f, "AgileReference({:?})", &self.0 .0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Is the space in .0 .0 intentional here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that too. It's rustfmt being creative. 🙄

}
}
2,489 changes: 411 additions & 2,078 deletions crates/libs/windows/src/core/bindings.rs

Large diffs are not rendered by default.

21 changes: 14 additions & 7 deletions crates/libs/windows/src/core/delay_load.rs
Expand Up @@ -8,13 +8,20 @@ use bindings::*;
/// # Safety
///
/// * Both the library and function names must be valid PCSTR representations
pub unsafe fn delay_load(library: PCSTR, function: PCSTR) -> Result<*mut core::ffi::c_void> {
let library = LoadLibraryA(library)?;
pub unsafe fn delay_load(library: PCSTR, function: PCSTR) -> Result<*const core::ffi::c_void> {
let library = LoadLibraryA(library);

if let Some(address) = GetProcAddress(library, function) {
Ok(address as _)
} else {
FreeLibrary(library);
Err(Error::from_win32())
if library == 0 {
return Err(Error::from_win32());
}

let address = GetProcAddress(library, function);

if !address.is_null() {
return Ok(address);
}

let result = Err(Error::from_win32());
FreeLibrary(library);
result
}
19 changes: 12 additions & 7 deletions crates/libs/windows/src/core/error.rs
Expand Up @@ -20,13 +20,13 @@ impl Error {
let function: RoOriginateError = core::mem::transmute(function);
function(code, core::mem::transmute_copy(&message));
}
let info = GetErrorInfo(0).and_then(|e| e.cast()).ok();
let info = GetErrorInfo().and_then(|e| e.cast()).ok();
Self { code, info }
}
}

pub fn from_win32() -> Self {
unsafe { Self { code: GetLastError().into(), info: None } }
unsafe { Self { code: HRESULT::from_win32(GetLastError()), info: None } }
}

/// The error code describing the error.
Expand Down Expand Up @@ -65,10 +65,10 @@ impl Error {
impl core::convert::From<Error> for HRESULT {
fn from(error: Error) -> Self {
let code = error.code;
let info = error.info.and_then(|info| info.cast().ok());
let info: Option<IErrorInfo> = error.info.and_then(|info| info.cast().ok());

unsafe {
let _ = SetErrorInfo(0, info.as_ref());
let _ = SetErrorInfo(0, info.abi());
}

code
Expand All @@ -93,7 +93,7 @@ impl core::convert::From<core::convert::Infallible> for Error {

impl core::convert::From<HRESULT> for Error {
fn from(code: HRESULT) -> Self {
let info: Option<IRestrictedErrorInfo> = unsafe { GetErrorInfo(0).and_then(|e| e.cast()).ok() };
let info: Option<IRestrictedErrorInfo> = GetErrorInfo().and_then(|e| e.cast()).ok();

if let Some(info) = info {
// If it does (and therefore running on a recent version of Windows)
Expand All @@ -108,7 +108,7 @@ impl core::convert::From<HRESULT> for Error {
return Self { code, info: Some(info) };
}

if let Ok(info) = unsafe { GetErrorInfo(0) } {
if let Ok(info) = GetErrorInfo() {
let message = unsafe { info.GetDescription().unwrap_or_default() };
Self::new(code, HSTRING::from_wide(message.as_wide()))
} else {
Expand All @@ -132,4 +132,9 @@ impl core::fmt::Display for Error {

impl std::error::Error for Error {}

type RoOriginateError = extern "system" fn(code: HRESULT, message: core::mem::ManuallyDrop<HSTRING>) -> BOOL;
type RoOriginateError = extern "system" fn(code: HRESULT, message: core::mem::ManuallyDrop<HSTRING>) -> i32;

fn GetErrorInfo() -> Result<IErrorInfo> {
let mut result = std::mem::MaybeUninit::zeroed();
unsafe { bindings::GetErrorInfo(0, result.as_mut_ptr()).from_abi(result) }
}
3 changes: 2 additions & 1 deletion crates/libs/windows/src/core/guid.rs
Expand Up @@ -17,7 +17,8 @@ pub struct GUID {
impl GUID {
/// Creates a unique `GUID` value.
pub fn new() -> Result<Self> {
unsafe { CoCreateGuid() }
let mut result = Self::zeroed();
unsafe { CoCreateGuid(&mut result).and_then(|| result) }
}

/// Creates a `GUID` represented by the all-zero byte-pattern.
Expand Down
9 changes: 3 additions & 6 deletions crates/libs/windows/src/core/heap.rs
Expand Up @@ -8,7 +8,7 @@ use bindings::*;
/// This function will fail in OOM situations, if the heap is otherwise corrupt,
/// or if getting a handle to the process heap fails.
pub fn heap_alloc(bytes: usize) -> Result<*mut core::ffi::c_void> {
let ptr = unsafe { HeapAlloc(GetProcessHeap()?, HEAP_NONE, bytes) };
let ptr = unsafe { HeapAlloc(GetProcessHeap(), 0, bytes) };

if ptr.is_null() {
Err(E_OUTOFMEMORY.into())
Expand All @@ -26,14 +26,11 @@ pub fn heap_alloc(bytes: usize) -> Result<*mut core::ffi::c_void> {

/// Free memory allocated by `HeapAlloc` or `HeapReAlloc`.
///
/// The pointer is allowed to be null. If there is an error getting the process heap,
/// the memory will be leaked.
/// The pointer is allowed to be null.
///
/// # Safety
///
/// `ptr` must be a valid pointer to memory allocated by `HeapAlloc` or `HeapReAlloc`
pub unsafe fn heap_free(ptr: *mut core::ffi::c_void) {
if let Ok(heap) = GetProcessHeap() {
HeapFree(heap, HEAP_NONE, ptr);
}
HeapFree(GetProcessHeap(), 0, ptr);
}
7 changes: 6 additions & 1 deletion crates/libs/windows/src/core/hresult.rs
Expand Up @@ -86,11 +86,16 @@ impl HRESULT {
let mut message = HeapString(core::ptr::null_mut());

unsafe {
let size = FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, core::ptr::null(), self.0 as _, 0, PWSTR(core::mem::transmute(&mut message.0)), 0, None);
let size = FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, std::ptr::null(), self.0 as _, 0, PWSTR(core::mem::transmute(&mut message.0)), 0, std::ptr::null());

HSTRING::from_wide(wide_trim_end(core::slice::from_raw_parts(message.0 as *const u16, size as usize)))
}
}

/// Maps a Win32 error code to an HRESULT value.
pub(crate) fn from_win32(error: u32) -> Self {
Self(if error == 0 { 0 } else { (error & 0x0000_FFFF) | (7 << 16) | 0x8000_0000 } as _)
}
}

unsafe impl Abi for HRESULT {
Expand Down
12 changes: 8 additions & 4 deletions crates/libs/windows/src/core/waiter.rs
Expand Up @@ -2,14 +2,18 @@ use super::*;
use bindings::*;

#[doc(hidden)]
pub struct Waiter(HANDLE);
pub struct WaiterSignaler(HANDLE);
pub struct Waiter(isize);
pub struct WaiterSignaler(isize);

impl Waiter {
pub fn new() -> Result<(Waiter, WaiterSignaler)> {
unsafe {
let handle = CreateEventA(None, true, false, None)?;
Ok((Waiter(handle), WaiterSignaler(handle)))
let handle = CreateEventW(std::ptr::null(), 1, 0, std::ptr::null());
if handle == 0 {
Err(Error::from_win32())
} else {
Ok((Waiter(handle), WaiterSignaler(handle)))
}
}
}
}
Expand Down
10 changes: 0 additions & 10 deletions crates/tools/bindings/Cargo.toml

This file was deleted.

84 changes: 0 additions & 84 deletions crates/tools/bindings/src/main.rs

This file was deleted.

2 changes: 1 addition & 1 deletion crates/tools/yml/src/main.rs
Expand Up @@ -173,7 +173,7 @@ jobs:
runs-on: windows-2019
strategy:
matrix:
generator: [bindings, windows, sys, yml]
generator: [windows, sys, yml]
steps:
- name: Checkout
uses: actions/checkout@v2
Expand Down