microsoft / windows-rs

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

`TranslateMessage` falsely gives error #3145

Closed JunkuiZhang closed 2 days ago

JunkuiZhang commented 3 days ago

Summary

The code TranslateMessage(&msg).ok() will give Err(), which is wrong.

Crate manifest

No response

Crate code

No response

JunkuiZhang commented 3 days ago

Screenshot 2024-07-04 160412

The error message is "Operation successful";

JunkuiZhang commented 3 days ago

By the way, I noticed some chages from isize to *mut cvoid, for example, HWND, HANDLE, HMONOTOR, HMODULE. And this change makes them thread unsafe, I have to manually unsafe impl Sync Send ... for them. Is this intended?

tim-weis commented 2 days ago

The code TranslateMessage(&msg).ok() will give Err(), which is wrong.

No, that is correct. The return value of TranslateMessage indicates whether the API processed the message. It is not a success/error discriminator. The issue is converting the BOOL to a Result<T> via ok().

The conventional use pattern for this API is to check whether the message was processed and pass it on to DispatchMessageW() if not, something like this:

if !TranslateMessage(&msg).as_bool() {
    DispatchMessageW(&msg);
}
riverar commented 2 days ago

Thanks Tim, beat me by a few seconds!

By the way, I noticed some chages from isize to *mut cvoid, for example, HWND, HANDLE, HMONOTOR, HMODULE. And this change makes them thread unsafe, I have to manually unsafe impl Sync Send ... for them. Is this intended?

This was a recent change (https://github.com/microsoft/windows-rs/pull/3111). This now matches the Rust standard handle type std::os::windows::raw::HANDLE.

riverar commented 2 days ago

I do wonder if there's any value in having an .ok() impl for BOOL, given it unexpectedly calls GetLastError which could contain anything and confuse folks further.

JunkuiZhang commented 2 days ago

@tim-weis Thank you for your reply. The .ok() really confuses here, I thought it checks BOOL and reports Err if return false for me.

This was a recent change (#3111). This now matches the Rust standard handle type std::os::windows::raw::HANDLE.

Thanks for that information. And, my idea is that the HWND, HANDLE and all these should be thread safe, right? So, can windows-rs unsafe impl Send + Sync for them, so I dont have to impl myself?