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

Tweak Error representation #614

Merged
merged 17 commits into from
Mar 7, 2025
Merged

Tweak Error representation #614

merged 17 commits into from
Mar 7, 2025

Conversation

newpavlov
Copy link
Member

@newpavlov newpavlov commented Mar 3, 2025

On UEFI targets Error is now represented as NonZeroUsize and on other targets as NonZeroI32. In the former case OS errors have the highest bit set to 1, while in the latter case OS errors are represented as negative integers.

Additionally, this PR removes Error::INTERNAL_START and Error::CUSTOM_START associated constants since the constants are no longer relevant and the inner representation is an internal detail. This is technically a breaking change, but I highly doubt that downstream users rely on them since they can not be used for anything useful, so we can consider it as a "fix" of unintentionally exported constants.

Closes #568

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@newpavlov
Copy link
Member Author

@josephlr
Note that I plan to release v0.3.2. after merging this (with a separate release PR), so if you have any objections or suggestions about the recently merged PR it's worth to sort them out before that.

@josephlr
Copy link
Member

josephlr commented Mar 4, 2025

@josephlr Note that I plan to release v0.3.2. after merging this (with a separate release PR), so if you have any objections or suggestions about the recently merged PR it's worth to sort them out before that.

That sounds like a good plan, I'm currently working through some of the recently merged PRs. I should hopefully be done by tomorrow. Thanks for all the fixes and reviews. Sorry I haven't had a huge amount of time for this recently.

Copy link
Member

@josephlr josephlr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this change, i have some suggestions on how we might make this easier to follow

@josephlr
Copy link
Member

josephlr commented Mar 5, 2025

@josephlr Note that I plan to release v0.3.2. after merging this (with a separate release PR), so if you have any objections or suggestions about the recently merged PR it's worth to sort them out before that.

That sounds like a good plan, I'm currently working through some of the recently merged PRs. I should hopefully be done by tomorrow. Thanks for all the fixes and reviews. Sorry I haven't had a huge amount of time for this recently.

I didn't quite get to all of the PRs today, but a quick skim shows nothing glaringly bad. I have a few ideas for PRs that might clean things up, but nothing that should block release.

@newpavlov newpavlov requested a review from josephlr March 5, 2025 16:56
Copy link
Member

@josephlr josephlr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, do we want to make the changes to efi_rng.rs in this PR, or in a followup PR.

@newpavlov
Copy link
Member Author

newpavlov commented Mar 7, 2025

do we want to make the changes to efi_rng.rs in this PR

I added commits which change efi_rng appropriately (I thought I already did it, but I guess it has slipped my mind for some reason).

@newpavlov newpavlov merged commit f2ede81 into master Mar 7, 2025
70 checks passed
@newpavlov newpavlov deleted the error_tweak branch March 7, 2025 09:37
@newpavlov newpavlov mentioned this pull request Mar 7, 2025
newpavlov added a commit that referenced this pull request Mar 17, 2025
### Added
- `efi_rng` opt-in backend [#570]
- `linux_raw` opt-in backend [#572]
- `.cargo/config.toml` example in the crate-level docs [#591]
- `getrandom_test_linux_without_fallback` configuration flag to test
that file fallback
  is not triggered in the `linux_android_with_fallback` backend [#605]
- Built-in support for `*-linux-none` targets [#618]
- Cygwin support [#626]

### Changed
- Update `wasi` dependency to v0.14 [#594]
- Add `#[inline]` attribute to the inner functions [#596]
- Update WASI and Emscripten links in the crate-level docs [#597]
- Do not use `dlsym` on MUSL targets in the
`linux_android_with_fallback` backend [#602]
- Remove `linux_android.rs` and use `getrandom.rs` instead [#603]
- Always use `RtlGenRandom` on Windows targets when compiling with
pre-1.78 Rust [#610]
- Internal representation of the `Error` type [#614]
- Remove `windows-targets` dependency and use [`raw-dylib`] directly
[#627]

### Removed
- `Error::INTERNAL_START` and `Error::CUSTOM_START` associated constants
[#614]

[#570]: #570
[#572]: #572
[#591]: #591
[#594]: #594
[#596]: #596
[#597]: #597
[#602]: #602
[#603]: #603
[#605]: #605
[#610]: #610
[#614]: #614
[#618]: #618
[#626]: #626
[#627]: #627
[`raw-dylib`]:
https://doc.rust-lang.org/reference/items/external-blocks.html?highlight=link#dylib-versus-raw-dylib
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.

Error on targets where RawOsError is not equal to i32
2 participants