NTSTATUS values are erroneously transformed into Ok(())

[chyyran]

Summary

NTSTATUS values that are not STATUS_SUCCESS (0x0) are transformed into Ok rather than Err.

Crate manifest

windows = { version = "0.51.0", features = ["Win32_Foundation"] }

Crate code

This test should pass, but it fails instead.

#[cfg(test)]
mod test {
    #[test]
    pub fn status_pending() {
        assert!(matches!(windows::Win32::Foundation::STATUS_PENDING.ok(), Err(_)))
    }
}

assertion failed: matches!(windows :: Win32 :: Foundation :: STATUS_PENDING.ok(), Err(_))

[chyyran]

This is a tangentially related request, but I think it's also more than reasonable to turn off the transformation of NTSTATUS to windows::core::Error for APIs that return NTSTATUS. It's a lot more work to unwrap Result<()> than to simply check STATUS_SUCCESS (and codes with STATUS_SEVERITY_SUCCESS that are not STATUS_SUCCESS may still require additional work), than to call .ok()? on an NTSTATUS.

This could be a compromise between transforming return codes with STATUS_SEVERITY_SUCCESS into Ok(()).

[ChrisDenton]

I don't think STATUS_PENDING should be an error. I think we should stick to the C++ semantics (albeit encoded in Rust types) rather than inventing our own. Otherwise we're going to get issues in places where the semantics don't quite match.

I prefer the suggestion to just return NTSTATUS. The only alternative that preserves the full semantics would be Result<NTSTATUS, NTSTATUS> which seems less ideal to me, though having a method on NTSTATUS that does this would be fine, imho. In any case, I think we would still want a method that performs the transformation into core::Error so that it can bubble up into more general code.

[kennykerr]

Note that the Win32 metadata has a way to deal with this through the CanReturnMultipleSuccessValues attribute but this is applied in a whack-a-mole fashion so I'm not too fond of it. If alternative success codes like STATUS_PENDING are more common with functions returning NTSTATUS then I'm fine with disabling the transformation for such signatures.

Note also that NTSTATUS provides the ok() method so that you can easily transform it into a Result if needed.