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

Prepares HRESULT to transparently store NTSTATUS values #989

Closed
wants to merge 2 commits into from
Closed

Prepares HRESULT to transparently store NTSTATUS values #989

wants to merge 2 commits into from

Conversation

tim-weis
Copy link
Contributor

Introducing idiomatic Rust error handling for NTSTATUS values requires two steps:

  1. Allowing HRESULT to transparently encode NTSTATUS values.
  2. Extending/augmenting/enlightening the generated NTSTATUS struct.

This PR proposes a solution to solving the first (for the moment). The core idea revolves around the fact that any NTSTATUS value can be mapped to an HRESULT value, that is distinct from any other HRESULT value. This makes HRESULT the perfect container to store any Win32 system error code, NTSTATUS, or (regular) HRESULT.

The following changes were done:

  • Added HRESULT::from_ntstatus():
    This turns NTSTATUS values into HRESULTs, similar to from_win32(). Outside of testing, this is not used, but rather is in preparation of a future impl From<NTSTATUS> for windows::Result.
  • Added HRESULT::ntstatus_code():
    Similar to win32_code() verifies whether the stored value is an NTSTATUS value and, if so, returns the decoded NTSTATUS value.
  • Adjusted HRESULT::message():
    Since FormatMessage can only be used with (regular) HRESULTs, this function produces a generic message for NTSTATUS values (e.g. "NTSTATUS: 0xC0000005").
  • std::fmt::Debug for Error did not need adjustments. In case of an NTSTATUS, the message already contains the decoded value and an additional debug field is not required.

If that all looks reasonable, I would try working on the second part. For that I would need a bit of guidance, though (e.g. would this be best done in gen_replacement() or gen_extensions(), and so on).

@tim-weis tim-weis mentioned this pull request Jul 20, 2021
@kennykerr
Copy link
Collaborator

Sorry I didn't get a chance to comment further on #983 - been quite sick the last few days - will take a look at this soon.

@tim-weis
Copy link
Contributor Author

I'm sad to hear that. Hope you get all the sleep and rest you need for a quick recovery.

@kennykerr kennykerr closed this Jul 22, 2021
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 this pull request may close these issues.

2 participants