os: Fix "double negation" in HRESULT hexadecimal formatting
#91
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DBouma
approved these changes
Nov 17, 2025
We typically see errors formatted as `-0x80004005`, because we tell
`pad_integral()` that the number is negative while we format-printed
it as **unsigned** integer which cannot be negative as represented by
the highest `0x80000000` bit being set - appearing as a double negation
of sorts? Either we remove the misleading `-` or print the number as
integer with the sign bit removed (which makes it harder to look up
online).
The end result is that we can and should just remove this strange
override of `LowerHex`, and rely purely on our `Display` implementation
of `HRESULT` that format-prints the inner `i32` as hexadecimal with `0x`
prefix, which automatically prints it as unsigned.
The end result is that we found one more case where `{:#x}` was
overridden to just `{:x}` because `LowerHex` is no longer implemented
for `HRESULT`, which was accidentally dropping the `0x` prefix in the
format string too.
MarijnS95
force-pushed
the
formatting-oopsies
branch
from
0989edc to
396534d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We typically see errors formatted as
-0x80004005, because we tellpad_integral()that the number is negative while we format-printed it as unsigned integer which cannot be negative as represented by the highest0x80000000bit being set - appearing as a double negation of sorts? Either we remove the misleading-or print the number as integer with the sign bit removed (which makes it harder to look up online).The end result is that we can and should just remove this strange override of
LowerHex, and rely purely on ourDisplayimplementation ofHRESULTthat format-prints the inneri32as hexadecimal with0xprefix, which automatically prints it as unsigned.The end result is that we found one more case where
{:#x}was overridden to just{:x}becauseLowerHexis no longer implemented forHRESULT, which was accidentally dropping the0xprefix in the format string too.