Print HRESULT description when formatting Error::Hr #718
Conversation
…ription is available.
teddemunnik
force-pushed
the
windows_format_hresult
branch
from
9aa9ca7 to
31fe87a
Compare
The FormatMessageW API expects a LPWSTR* disguising as a LPWSTR, in the case that the FORMAT_MESSAGE_ALLOCATE_BUFFER flag is present.
| | FORMAT_MESSAGE_IGNORE_INSERTS | ||
| | FORMAT_MESSAGE_MAX_WIDTH_MASK, | ||
| null(), | ||
| hr as u32, |
There was a problem hiding this comment.
Let's change this u32 to DWORD (from winapi::shared::minwindef). I know functionally they're the same and the u32 looks nicer but matching the types used in Microsoft docs when calling a winapi function will help whomever is reading this code in the future.
There was a problem hiding this comment.
Completely agree, changed it
…tch windows API types.
| null(), | ||
| hr as u32, | ||
| 0, | ||
| transmute(&mut message_buffer as *const LPWSTR), |
There was a problem hiding this comment.
I think we can remove the as *const LPWSTR part. Otherwise we might as well just write transmute(&message_buffer).
Probably best to have this instead:
transmute(&mut message_buffer)The mut part doesn't seem required for the code to work, but it communicates the fact that the function will modify the pointer so we can keep the mut.
There was a problem hiding this comment.
Sorry, I was a bit slow with this comment. I wasn't waiting until you push the DWORD change, I swear! 😬
There was a problem hiding this comment.
It seems to work without the as *const LPWSTR, but then clippy complains:
warning: transmute from a reference to a pointer
--> druid-shell\src\platform\windows\error.rs:55:13
|
55 | transmute(&mut message_buffer),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try:&mut message_buffer as *mut *mut u16 as *mut u16
|
= note:#[warn(clippy::useless_transmute)]on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_transmute
Is that another false positive? I already turned off clippy::crosspointer_transmute for this function in an earlier change.
There was a problem hiding this comment.
Clippy seems unhappy indeed. However as *mut LPWSTR seems to satisfy clippy and is more to the truth, so maybe use that.
There was a problem hiding this comment.
Ah just listening to clippy apparantly this just works: &mut message_buffer as *const LPWSTR as LPWSTR, didn't know you could do that without transmute. Then I can remove the clippy ignore flags. Does that sound good?
There was a problem hiding this comment.
Great find! I was trying to get rid of transmute earlier but didn't know how. It looks like the following works as well:
&mut message_buffer as *mut LPWSTR as LPWSTRLet's go with that if possible.
There was a problem hiding this comment.
Sounds good! I just pushed this change :)
teddemunnik
force-pushed
the
windows_format_hresult
branch
from
3faa134 to
5771906
Compare
|
I was going to complain about the obviously wrong casting, but that's winapi being winapi, and I agree this is the best way to write that in Rust. |
When implementing error handling for #712 I thought it'd be nice to add some details to the error
HRESULTerror messages instead of just error codes.This change adds a short description from the FormatMessage windows API when printing an
Error::Hr, when one is available.As an example the following messages:
are now displayed as: