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

TryEnterCriticalSection prototype does not match Windows docs #70504

Closed
RalfJung opened this issue Mar 28, 2020 · 3 comments · Fixed by #70510
Closed

TryEnterCriticalSection prototype does not match Windows docs #70504

RalfJung opened this issue Mar 28, 2020 · 3 comments · Fixed by #70510
Labels
C-bug Category: This is a bug. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

According to this documentation, the return type of TryEnterCriticalSection is BOOL (an alias for i32), but in Rust we use BOOLEAN (an alias for u8):

pub fn TryEnterCriticalSection(CriticalSection: *mut CRITICAL_SECTION) -> BOOLEAN;

Which one is right?

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 28, 2020
@Mark-Simulacrum
Copy link
Member

winapi uses BOOL (c_int/i32) -- I suspect that this has been wrong since forever. Presumably we haven't noticed because in practice the values are always just != 0 and usually not 0 modulo 256, either, so we aren't spuriously wrong, and the value is passed in a register -- which on ~all platforms is 32-64bits anyway.

Presumably though we should just fix this; cc @alexcrichton who originally added these in 10b103a and cc @rust-lang/libs more broadly.

(Also, can I just say that having both BOOLEAN and BOOL as aliases and they mean different things is pretty confusing?)

@alexcrichton
Copy link
Member

This is practically guaranteed to just be a typo on my end, so BOOL is most likely right.

@RalfJung
Copy link
Member Author

(Also, can I just say that having both BOOLEAN and BOOL as aliases and they mean different things is pretty confusing?)

Yeah, that one is on Microsoft... other functions do return BOOLEAN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants