Bug: couldn't call QueryDisplayConfig except QDC_DATABASE_CURRENT, with 0.40.0

[topia]

Which crate is this about?

windows

Crate version

0.40.0

Summary

I don't know that this is an issue by win32metadata, or by windows-rs, or by design.

By its definition, QueryDisplayConfig's last parameter must be NULL for QDC_DATABASE_CURRENT, but must not be NULL otherwise.

Current win32metadata (checked with 32.0.17-preview), it defined as [Out] DISPLAYCONFIG_TOPOLOGY_ID* currentTopologyId.
without microsoft/windows-rs#1939, I called the function with null_mut(). but for now, I don't know how I can call it.
I found I can call it with #[allow(deref_nullptr)] &mut *null_mut::<DISPLAYCONFIG_TOPOLOGY_ID>() while writing this issue. Is this the correct way?

I saw the conditional is used in its definition, but I don't believe win32metadata can utilize it.

Could you help me? I want to know a workaround and/or the long-term solution for that.

Toolchain version/configuration

Default host: x86_64-pc-windows-msvc
rustup home: D:\data\depot\rustup

installed toolchains

stable-x86_64-pc-windows-msvc
nightly-x86_64-pc-windows-msvc

installed targets for active toolchain

aarch64-apple-darwin
aarch64-apple-ios
aarch64-linux-android
aarch64-unknown-linux-gnu
nvptx64-nvidia-cuda
wasm32-unknown-emscripten
wasm32-unknown-unknown
wasm32-wasi
x86_64-apple-darwin
x86_64-pc-windows-gnu
x86_64-pc-windows-msvc
x86_64-unknown-linux-gnu
x86_64-unknown-linux-musl

active toolchain

stable-x86_64-pc-windows-msvc (default)
rustc 1.63.0 (4b91a6ea7 2022-08-08)

Reproducible example

use std::ptr::null_mut;
use windows::core::HRESULT;
use windows::Win32::Devices::Display::{DISPLAYCONFIG_MODE_INFO, DISPLAYCONFIG_PATH_INFO, GetDisplayConfigBufferSizes, QueryDisplayConfig};
use windows::Win32::Foundation::{ERROR_INSUFFICIENT_BUFFER, ERROR_SUCCESS, WIN32_ERROR};
use windows::Win32::Graphics::Gdi::QDC_ONLY_ACTIVE_PATHS;

pub fn main() -> windows::core::Result<()> {
    let flags = QDC_ONLY_ACTIVE_PATHS;
    loop {
        let mut num_path_array_elements = u32::MAX;
        let mut num_mode_info_array_elements = u32::MAX;
        let ret: WIN32_ERROR = unsafe {
            std::mem::transmute(GetDisplayConfigBufferSizes(
                flags,
                &mut num_path_array_elements,
                &mut num_mode_info_array_elements,
            ))
        };
        if ret != ERROR_SUCCESS {
            break Err(HRESULT::from(ret).into());
        }
        let mut paths =
            Vec::<DISPLAYCONFIG_PATH_INFO>::with_capacity(num_path_array_elements as usize);
        let mut modes =
            Vec::<DISPLAYCONFIG_MODE_INFO>::with_capacity(num_mode_info_array_elements as usize);
        let ret: WIN32_ERROR = unsafe {
            std::mem::transmute(QueryDisplayConfig(
                flags,
                &mut num_path_array_elements,
                paths.as_mut_ptr(),
                &mut num_mode_info_array_elements,
                modes.as_mut_ptr(),
                null_mut(),  // this line
            ))
        };
        if ret == ERROR_INSUFFICIENT_BUFFER {
            continue;
        } else if ret != ERROR_SUCCESS {
            break Err(HRESULT::from(ret).into());
        }
        unsafe {
            paths.set_len(num_path_array_elements as usize);
            modes.set_len(num_mode_info_array_elements as usize);
        }
        for path in paths {
            println!("path_info: {:?}", path.flags)
        }
        for mode in modes {
            println!("mode_info: {:?}: {:?}", mode.id, mode.infoType)
        }
        break Ok(());
    }
}

Crate manifest

[dependencies.windows]
version = "0.40"
features = [
    "Win32_Devices_Display",
    "Win32_Foundation",
    "Win32_Graphics_Gdi",
]

Expected behavior

Can compile and run correctly.

Actual behavior

failed to compile with expected &mut DISPLAYCONFIG_TOPOLOGY_ID, found *-ptr error.

Additional comments

By the way, using ref over pointer is an awesome change. Thank you for your work and I'm migrating it happily.

[riverar]

Not sure I understand the bug. Here's the output I get:

warning: unused import: `DISPLAYCONFIG_TOPOLOGY_ID`
 --> src\main.rs:3:90
  |
3 | use windows::Win32::Devices::Display::{DISPLAYCONFIG_MODE_INFO, DISPLAYCONFIG_PATH_INFO, DISPLAYCONFIG_TOPOLOGY_ID, GetDisplayConfigBuffe...
  |                                                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: `app` (bin "app") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
     Running `target\debug\app.exe`
path_info: 1
mode_info: 37125: DISPLAYCONFIG_MODE_INFO_TYPE(2)
mode_info: 0: DISPLAYCONFIG_MODE_INFO_TYPE(1)

[topia]

Thank you for your attention to reproducing this.
I believe you're using 0.39 to try.
I fixed my manifest snippet to request 0.40, and I pushed a whole tree to reproduce it.
edit: additionally, I fixed the unused_imports.

[riverar]

Thanks @topia. Looks like metadata is missing an [Optional] attribute on that last [Out] parameter (due to not parsing SAL conditionals like you mentioned).

WINUSERAPI
_Success_(return == ERROR_SUCCESS) LONG
WINAPI
QueryDisplayConfig(
    _In_ UINT32 flags,
    _Inout_ UINT32* numPathArrayElements,
    _Out_writes_to_(*numPathArrayElements, *numPathArrayElements) DISPLAYCONFIG_PATH_INFO* pathArray,
    _Inout_ UINT32* numModeInfoArrayElements,
    _Out_writes_to_(*numModeInfoArrayElements, *numModeInfoArrayElements) DISPLAYCONFIG_MODE_INFO* modeInfoArray,
+   _When_(!(flags & QDC_DATABASE_CURRENT), _Pre_null_)
+   _When_(flags & QDC_DATABASE_CURRENT, _Out_)
        DISPLAYCONFIG_TOPOLOGY_ID* currentTopologyId);

Will transfer this to win32metadata.

@kennykerr Heads up, others may hit this elsewhere and we'll need some sort of workaround/story.

[kennykerr]

Yes, I think I'll have to remove the pointer to reference conversion in Rust as these sorts of metadata bugs are pretty widespread.

[kennykerr]

As of microsoft/windows-rs#2036 you can now once again call this API and pass a null pointer. I'll leave this issue to deal with the SAL parsing bugs.

[mikebattista]

From @sotteson1, parsing conditionals is problematic but we can investigate adding a nullable annotation if we detect null SAL annotations. See microsoft/CsWin32#130 for requests for this from C#/Win32.