microsoft / windows-rs

Rust for Windows
https://kennykerr.ca/rust-getting-started/
Apache License 2.0
10.36k stars 486 forks source link

IMMDevice::GetState() requires an out param and returns HRESULT, but the type of the return value is DEVICE_STATE #3067

Closed docwilco closed 4 months ago

docwilco commented 4 months ago

Summary

The Rust IMMDevice::GetState takes an "out" parameter (pdwstate: *mut u32), and returns a value of type DEVICE_STATE:

    pub unsafe fn GetState(&self, pdwstate: *mut u32) -> DEVICE_STATE {
        (::windows_core::Interface::vtable(self).GetState)(::windows_core::Interface::as_raw(self), pdwstate)
    }

The VTable implementation for that is:

    pub GetState: unsafe extern "system" fn(*mut ::core::ffi::c_void, *mut u32) -> DEVICE_STATE,

This looks wrong to me, as the C++ side of things defines IMMDevice::GetState as:

HRESULT GetState(
  [out] DWORD *pdwState
);

This makes me think the Rust definition should be:

    pub unsafe fn GetState(&self) -> DEVICE_STATE {
        let mut result__ = core::mem::zeroed();
        (::windows_core::Interface::vtable(self).GetState)(::windows_core::Interface::as_raw(self), &mut result__).map(|| DEVICE_STATE(result__))
    }

With the VTable entry being:

    pub GetState: unsafe extern "system" fn(*mut core::ffi::c_void, *mut u32) -> windows_core::HRESULT,

I'm not super familiar with the windows-rs crate internals just yet, so I'm not sure if this is generated code where the generator should be updated, or if this is code that should now be updated manually?

Crate manifest

No response

Crate code

No response

riverar commented 4 months ago

We fixed this on the metadata side already, but haven't ingested that new metadata yet on the Rust side. See also: https://github.com/microsoft/win32metadata/issues/1856#issuecomment-2059966655

kennykerr commented 4 months ago

Yep, I'm just waiting for a release of the Win32 metadata.

docwilco commented 4 months ago

Oooh, shiny! Thank you! <3