Skip to content

Conversation

nicholasbishop
Copy link
Member

The overall diff here is fairly large, somewhat unavoidable since there are so many functions in BootServices. I've broken it down into smaller commits that should make it clearer how the transformations were done, and also easier to bisect if any bugs are found later.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@@ -94,7 +94,7 @@ struct BootServicesInternal {

// Protocol handlers
install_protocol_interface: unsafe extern "efiapi" fn(
handle: &mut Option<Handle>,
handle: *mut uefi_raw::Handle,
Copy link
Member

@phip1611 phip1611 Jun 19, 2023

Choose a reason for hiding this comment

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

Why can't we use Option<&mut uefi_raw::Handle> here and in similar places? It has the same memory layout as a raw pointer, but we do have more guarantees in higher-level Rust APIs.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could, but I'm wary of using any references at all in the uefi-raw interface. Broadly, my thinking is that we want uefi-raw to be as close to the C interface as possible, and in particular placing no restrictions on the API that wouldn't also be present for a C user. My understanding is that a Rust reference, even if it's never dereferenced, must always be valid (aligned, non-dangling), and obey the (currently unfinished) aliasing rules.

@nicholasbishop nicholasbishop force-pushed the bishop-uefi-raw-boot-services-5 branch from e073a4c to 59da9b3 Compare June 19, 2023 15:35
@nicholasbishop nicholasbishop merged commit 926dce7 into rust-osdev:main Jun 19, 2023
@nicholasbishop nicholasbishop deleted the bishop-uefi-raw-boot-services-5 branch June 19, 2023 15:43
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.

2 participants