NTSTATUS error codes

[tim-weis]

At this time, both standard system error codes and HRESULTs are well covered by the Windows crate. This is not the case for NTSTATUS based error codes. Those are rarely used in the Windows API surface, though I just ran into a set of functions that do, namely in the Cryptography API: Next Generation (e.g. BCryptOpenAlgorithmProvider).

Error handling with those APIs is manual, and with NTSTATUS being a generated type, interoperability across crate boundaries is stifled. Are there any plans to lift NTSTATUS out of the generated code into Windows' Error type?

[kennykerr]

Ooh, I love BCrypt. 😉

So there are a few different angles to this.

The general principle is that if a type exists in both the WinRT and Win32 type systems then it must necessarily be lifted out and defined canonically in the Windows crate. NTSTATUS is not such a type so it can remain where it is.

However, NTSTATUS is a common error type so it makes sense to enlighten it so that callers can easily convert it to a Result if needed. This is what I did for BOOL and I think that works well enough. In the case of NTSTATUS, there's no ambiguity over whether it is in fact representing a status code so it could even provide a more seamless experience.

As far as interoperability across crates, that should be solved with #432 however I would suggest that libraries should prefer to convert error codes like WIN32_ERROR and NTSTATUS into an HRESULT so that it can be portably and uniformly treated as a windows::Result. An idiomatic Rust wrapper library for BCrypt would not expose NTSTATUS error codes as at all.

[tim-weis]

Ooh, I love BCrypt. 😉

Oh wow, another phenomenal installment? That was a great read, well paced, well delivered, covering everything from the overall architecture, the rationale behind it, recurring patterns, down to common use cases. Plus a few "Cryptography for Dummies" lessons by the side. Thank you, Kenny! As a teacher you are extraordinarily effective (and this isn't just me either).

The general principle is that if a type exists in both the WinRT and Win32 type systems then it must necessarily be lifted out and defined canonically in the Windows crate.

This wasn't obvious to me until you spelled it out. That makes sense, thanks!

However, NTSTATUS is a common error type so it makes sense to enlighten it so that callers can easily convert it to a Result if needed. This is what I did for BOOL and I think that works well enough.

Agreed.

In the case of NTSTATUS, there's no ambiguity over whether it is in fact representing a status code so it could even provide a more seamless experience.

Just to verify that I fully understand this: While a BOOL can represent any piece of information, a developer is required to explicitly convert it to a Result by calling ok() where applicable. With NTSTATUS that conversion could be implicit, allowing a client to write BCryptHashData()?; instead. Does that roughly capture the gist of your statement?

I would suggest that crates should prefer to convert error codes like WIN32_ERROR and NTSTATUS into an HRESULT so that it can be portably and uniformly treated as a windows::Result.

Up until yesterday I was convinced that going from an NTSTATUS to an HRESULT would necessarily be lossy. As it turns out, HRESULTs can absolutely represent NTSTATUS values and distinguish between either one (bit N).

Repurposing the HRESULT wrapped by Error for NTSTATUS values would still necessitate changes to HRESULT's interface so that clients can distinguish between 'true' HRESULT values and NSTATUS values in disguise, when necessary. If you are open this type of change I can prepare a draft PR to discuss and explore possible implementations.

For completeness, the generated NTSTATUS type currently doesn't #[derive{PartialEq, Eq)] making it unavailable in match expressions. Is there a particular reason for this, or was this simply not added (yet) to the code generator?

[tim-weis]

Actually, the public interface of neither Error nor HRESULT would need to change (nor the internal representation). There only need to be private helper functions on HRESULT (fn is_ntstatus(&self) -> bool, and possibly fn ntstatus_code(&self) -> Option<u32>) to teach std::fmt::Debug for Error about NTSTATUS values, and optionally improve HRESULT::message().

The latter currently produces an empty string for NTSTATUS values (or the handful I verified anyway). I'm not aware of a (public) API that produces a human-readable error message from an NTSTATUS value. If there is no such API, it may be reasonable to have the previously mentioned private helpers on HRESULT moved into its public interface.

Regardless, I can still try to prepare a draft PR to illustrate, how NTSTATUS values could be incorporated into the current system, if desired. (PR #989)

[kennykerr]

Thanks Tim, I had a quick look at the PR. I like to work backward from the ideal design and then provide only as much library support as is absolutely necessary. The ideal in my mind is that we treat functions that return NTSTATUS the same as the code generator treats functions returning HRESULT. The generated Rust functions would just return Result<T> in both cases with appropriate return value transformations as needed. So developers wouldn't really need to think about the underlying type of the error code. I would expect minimal library changes but substantial code generator work. I do however need to do some refactoring first to make that easier. There is some technical debt there that needs to be paid back before I can afford to add much more functionality to the generator.

[kennykerr]

I'd love to do the same for BOOL-returning functions but that requires some metadata which doesn't exists.

microsoft/win32metadata#536