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

How to use HRESULT with functions returning i32 #603

Closed
kiron1 opened this issue Aug 2, 2021 · 4 comments · Fixed by #832
Closed

How to use HRESULT with functions returning i32 #603

kiron1 opened this issue Aug 2, 2021 · 4 comments · Fixed by #832

Comments

@kiron1
Copy link

kiron1 commented Aug 2, 2021

For example the function InitializeSecurityContextW returns an i32 where the return status code should be tested for SEC_E_OK which is of type HRESULT which is implemented as:

#[repr(transparent)]
pub struct HRESULT(pub u32);

Doing:

let status = InitializeSecurityContextW(...);
if status == SEC_E_OK.0 {
  todo!();
}

The above snippet will give compiler errors error[E0308]: mismatched types: since comparing i32 against u32.

Did there already a pattern/idiom emerge how to handle such situations, or am I missing something here? It could be a good addition to the FAQ.

@riverar
Copy link
Collaborator

riverar commented Aug 3, 2021

I'll chat with @kennykerr about this next week but given an HRESULT is represented with 31 bits and not 32, I'm leaning towards pub struct HRESULT(pub u32); being a bug. Seems to be an artifact of a recent conversion from Win32 error codes microsoft/windows-rs@80846ad

@kiron1
Copy link
Author

kiron1 commented Aug 4, 2021

Thanks for the background information @riverar.

In the meantime I use this pattern (maybe helpful for others as well):

let status = InitializeSecurityContextW(...);
let status = HRESULT(status as _);
...

@kennykerr
Copy link
Contributor

There are two different issues here.

  1. InitializeSecurityContextW is actually defined as returning a SECURITY_STATUS and various SEC_XXXX codes are defined as SECURITY_STATUS values including SEC_E_OK. Other SEC_XXXX values are however defined simply as HRESULT values. The Win32 metadata will need to figure out how to handle this. The workaround here seems perfectly reasonable.

  2. HRESULT is really just a series of bits and all 32 bits are required. The Windows crate thus represents this with u32 because it is easier to check individual bit values. Ideally all APIs that return HRESULT values would actually use HRESULT and not i32. If we find that SECURITY_STATUS is really just a set of HRESULT values then perhaps this API could return HRESULT instead. That seems to be the case and would simplify its usage considerably.

Transferring this issue to the win32 metadata repo.

@kennykerr kennykerr transferred this issue from microsoft/windows-rs Aug 6, 2021
@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Aug 6, 2021

various SEC_XXXX codes are defined as SECURITY_STATUS values including SEC_E_OK

SEC_E_INSUFFICIENT_MEMORY is defined as ((SECURITY_STATUS)0x1300) in <IssPer16.h> but _HRESULT_TYPEDEF_(0x80090300L) in <winerror.h>. Other SEC_… macros likewise have different values between these two files. <IssPer16.h> is included from <security.h> if ISSP_LEVEL == 16, but ISSP_LEVEL is defined only in <sspi.h>, where the value is always 32. I guess <IssPer16.h> is only for 16-bit operating systems and SECURITY_STATUS in win32 is the same as HRESULT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants