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

GlobalUnlock return value seems weird #1770

Open
knopp opened this issue Dec 17, 2023 · 5 comments
Open

GlobalUnlock return value seems weird #1770

knopp opened this issue Dec 17, 2023 · 5 comments
Labels
rust Critical for Rust adoption

Comments

@knopp
Copy link

knopp commented Dec 17, 2023

Summary

It is translated to a result, but in both case GetLastError returns NO_ERROR so it seems a bit weird.

https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-globalunlock

Crate manifest

No response

Crate code

No response

@knopp knopp added the bug Something isn't working label Dec 17, 2023
@kennykerr
Copy link
Contributor

There are cases where the function can fail, in which case GetLastError reports that information.

@kennykerr kennykerr added question Further information is requested and removed bug Something isn't working labels Dec 17, 2023
@knopp
Copy link
Author

knopp commented Dec 17, 2023

The problem is that false in case of GlobalUnlock does not necessarily mean that error has happened, but windows-rs will return Error unconditionally, even if GetLastError is 0. So something like

GlobalUnlock(hglobal)?

will abort calling function with an error, which is in fact a success.

@tim-weis
Copy link
Contributor

GlobalUnlock() doesn't follow the conventional error reporting pattern, and the Result<> transformation carries the surprises over into Rust.

The API should probably get the [CanReturnMultipleSuccessValues] or [CanReturnErrorsAsSuccess] attribute to suppress this transformation here. I don't recall the rationale behind choosing one over the other.

@kennykerr
Copy link
Contributor

Starting to think the default for the return value transformation is all wrong. The transformation should be opt-in rather than opt-out for Win32.

@kennykerr
Copy link
Contributor

Anyway, I'll transfer to the Win32 metadata repo for their consideration.

Rust only considers the CanReturnMultipleSuccessValues attribute.

@kennykerr kennykerr transferred this issue from microsoft/windows-rs Dec 18, 2023
@riverar riverar added rust Critical for Rust adoption and removed question Further information is requested labels Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Critical for Rust adoption
Projects
None yet
Development

No branches or pull requests

4 participants