-
-
Notifications
You must be signed in to change notification settings - Fork 169
Change Handle representation to be non-nullable so that Option<Handle> is FFI-safe #309
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good change to me, left a couple comments. Please also rebase the fixup commits into the first commit.
Also I've just realised that an identical change can be made for |
Oh wow on ut/doctest stage it does not know that such feature exists, no idea how to fix it besides reverting back to using |
Ah that is confusing. I think it's because that feature is part of To fix, gate the feature behind diff --git a/src/lib.rs b/src/lib.rs
index b12602b..035aee3 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -23,13 +23,12 @@
//! For example, a PC with no network card might not contain a network driver,
//! therefore all the network protocols will be unavailable.
-#![cfg_attr(feature = "exts", feature(allocator_api, alloc_layout_extra))]
+#![cfg_attr(feature = "exts", feature(allocator_api, alloc_layout_extra, vec_into_raw_parts))]
#![feature(auto_traits)]
#![feature(control_flow_enum)]
#![feature(try_trait_v2)]
#![feature(abi_efiapi)]
#![feature(negative_impls)]
-#![feature(vec_into_raw_parts)]
#![no_std]
// Enable some additional warnings and lints.
#![warn(missing_docs, unused)] |
Please rebase against latest master to resolve the conflict in boot.rs, and then I think it'll be good to merge. |
Right, totally forgot about the other MR |
Thanks, merging now. |
While I rewrote the OUT pointers in UEFI methods to use
MaybeUninit
(which actually looks very clean and logical) to avoid ever having null handles, the reason for the repr change is so that whenever an UEFI method accepts a handle - by the docs it can be optional or not, and this can be expressed by just having eitherHandle
orOption<Handle>
in the fn-pointer args, which is very clean and removes the need for null handles (Handle::unitialized()
) to ever exist on Rust side.I spent a night debugging my impls of (dis)connect_controller (that I will make a PR for too at some point) only to realize that I automatically assumed that whatever I did in the PR was already there ¯_(ツ)_/¯