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

rework crate to allocationless #131

Merged
merged 2 commits into from
Oct 31, 2022
Merged

rework crate to allocationless #131

merged 2 commits into from
Oct 31, 2022

Conversation

zdivelbiss
Copy link
Contributor

An overall rework of structures to make the crate allocationless. However, a new feature allocator_api has been added in addition to these changes, which allows use of PlatformInfo and some other allocating structures, by providing a core::alloc::Allocator in the constructor of these types. This allows semantic interfacing with existing allocation APIs, while also ensuring we aren't using the alloc crate allocation behaviour by default, as it isn't very extensible for use in bare-metal development contexts.

An overall rework of structures to make the crate allocationless.
However, a new feature `allocator_api` has been added in addition
to these changes, which allows use of `PlatformInfo` and some other
allocating structures, by providing a `core::alloc::Allocator` in the
constructor of these types. This allows semantic interfacing with
existing allocation APIs, while also ensuring we aren't using the
`alloc` crate allocation behaviour by default, as it isn't very
extensible for use in bare-metal development contexts.
Copy link
Member

@IsaacWoods IsaacWoods left a comment

Choose a reason for hiding this comment

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

Great stuff, thanks for persevering with this despite the Git troubles. This is looking mostly good, I've left some comments here and there (a few apply generally rather than to the specific places I've marked).

A few are nits but would really bring down the diff size, which would definitely be worth it because a few parts of this PR, which'll all end up in a single commit, have their subtleties.

acpi/src/address.rs Outdated Show resolved Hide resolved
acpi/src/lib.rs Outdated Show resolved Hide resolved
acpi/src/lib.rs Outdated Show resolved Hide resolved
acpi/src/lib.rs Show resolved Hide resolved
acpi/src/lib.rs Outdated Show resolved Hide resolved
acpi/src/lib.rs Outdated Show resolved Hide resolved
acpi/src/lib.rs Show resolved Hide resolved
acpi/src/lib.rs Outdated Show resolved Hide resolved
acpi/src/lib.rs Show resolved Hide resolved
acpi/src/madt.rs Show resolved Hide resolved
@zdivelbiss
Copy link
Contributor Author

Alright, unless you have more nits or changes you'd like, I believe this PR is all finished up.

Copy link
Member

@IsaacWoods IsaacWoods left a comment

Choose a reason for hiding this comment

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

Great, looks good to me! Thanks for seeing this through!

I think we've still got other changes we want to make before a release candidate yet right, but I can push to crates.io if you for testing?

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