Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork175
UEFI Allocator: Implementcoresallocator_api#1632
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coresallocator_apicoresallocator_apicoresallocator_apiNo need for this so far, but this provides downstream users moreflexibility.
uefi/src/allocator.rs Outdated
| #[cfg(feature = "unstable")] | ||
| unsafe impl core::alloc::Allocator for Allocator { | ||
| fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, core::alloc::AllocError> { | ||
| let ptr = unsafe { <Allocator as GlobalAlloc>::alloc(self, layout) }; |
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.
Do we need to do something here to handle ZSTs? I'm not familiar with the details of theAllocator trait, but I noticed this in thedocumentation:
In contrast to
GlobalAlloc,Allocatorallows zero-sized allocations. If an underlying allocator does not support this (like jemalloc) or responds by returning a null pointer (such aslibc::malloc), this must be caught by the implementation.
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.
Yes, I'm just not sure how to handle it :D but I can figure it out, I guess
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.
Nice use case but you're right. I added it in a dedicated commit
Uh oh!
There was an error while loading.Please reload this page.
We used std::alloc::System as template.[0]https://doc.rust-lang.org/src/std/alloc.rs.html#137
| @@ -53,6 +51,44 @@ mod bootservices { | |||
| } | |||
| unsafe { boot::free_pool(ptr) }.unwrap(); | |||
| } | |||
| /// Tests getting the memory map. | |||
| pub fn test_memory_map() { | |||
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.
Moving this function seems unrelated to the rest of the PR -- could you drop this change or move it to a separate PR?
| unsafe impl core::alloc::Allocator for Allocator { | ||
| fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, core::alloc::AllocError> { | ||
| // Stable alternative for Layout::dangling() | ||
| fn dangling_for_layout(layout: Layout) -> NonNull<u8> { |
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.
Since this is gated byunstable anyway, I think we might as well enablealloc_layout_extra and useLayout::dangling directly.
Closing for the same reason as discussed here:#1606 (comment) |
Uh oh!
There was an error while loading.Please reload this page.
Implement the allocator API (of
core) for the UEFI allocator.Checklist