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

GetMessage should return int #308

Closed
AArnott opened this issue Mar 5, 2021 · 5 comments
Closed

GetMessage should return int #308

AArnott opened this issue Mar 5, 2021 · 5 comments
Labels
usability Touch-up to improve the user experience for a language projection

Comments

@AArnott
Copy link
Member

AArnott commented Mar 5, 2021

There are APIs that return more than two values (most prominently GetMessage), in case thats relevant to the decision

Originally posted by @weltkante in #301 (comment)

From GetMessage docs:

Because the return value can be nonzero, zero, or -1, avoid code like this:

This is a bit hard to follow, since it apparently doesn't classify -1 as "nonzero", which in fact it is.
How about we break away from the metadata and force this method to return int since it doesn't limit its return value to just zero and non-zero? In fact the docs themselves warn against code like this: while (GetMessage( lpMsg, hWnd, 0, 0)), so forcing the return type to be int would help guide folks to follow the docs more.

@mikebattista mikebattista added the usability Touch-up to improve the user experience for a language projection label Mar 9, 2021
@sotteson1
Copy link
Contributor

sotteson1 commented Mar 25, 2021

I'm not sure. A BOOL doesn't (or shouldn't) restrict itself to 1 or 0. It's an int, just like the Win32 BOOL.

@kennykerr
Copy link
Contributor

Yes, there are a few functions that do this and BOOL already leaves room for it.

Incidentally, I generally just ignore the result of GetMessage. It's simpler to check the resulting message:

https://github.com/microsoft/windows-rs/blob/8d84c192afd45b6094d53eb23e6962b2b5b782b7/examples/clock/src/main.rs#L456-L460

@weltkante
Copy link

weltkante commented Mar 26, 2021

Incidentally, I generally just ignore the result of GetMessage.

Thats potentially a bug, or at least relying on an implementation detail, I don't know if GetMessage will zero out the MSG before erroring out. If not you'd repeatedly dispatch the same message if GetMessage keeps failing.

But don't worry, if you already only pass zero/null arguments then it can't fail anyways, since the arguments are trivially correct.

Its for when you do filtered message retrieval that you have to worry about error checking. If you don't and e.g. the HWND is destroyed then your message loop runs at 100% CPU and you keep dispatching potentially invalid messages.

@sotteson1
Copy link
Contributor

I'm going to close this because I think we should preserve the API signature the way it's defined. The way BOOL works in Win32 isn't obvious to those used to bool which is either 1 or 0, but it's how Win32 has always worked.

@AArnott
Copy link
Member Author

AArnott commented Mar 29, 2021

Ok. In CsWin32 I can make sure BOOL provides access to the underlying int while still allowing implicit conversion to bool based on a non-zero value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
usability Touch-up to improve the user experience for a language projection
Projects
None yet
Development

No branches or pull requests

5 participants