Skip to content

acpi: Remove Clone Copy traits for MADT #238

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

Merged
merged 1 commit into from
Jan 8, 2025
Merged

acpi: Remove Clone Copy traits for MADT #238

merged 1 commit into from
Jan 8, 2025

Conversation

mrjbom
Copy link
Contributor

@mrjbom mrjbom commented Jan 5, 2025

Copying and cloning a MADT does not copy Entries, so the cloned MADT is invalid.
But or maybe it's worth doing this: #223

@IsaacWoods
Copy link
Member

Hey @mrjbom thanks for opening this PR and for reporting the initial issue. As noted in #223 these types were originically Copy as it was required for them to be repr(packed) - this requirement seems to have been dropped at some point and is now obviously nothing but a footgun.

I think #223 is the right answer in the end as it is unsafe to move the Madt away from its entries in all cases, but is more of a breaking change. This would be a good middle-ground in the meantime - this is technically a breaking change but prevents behaviour that is unsound, so is fine as a semver bug version.

Thanks!

@IsaacWoods IsaacWoods merged commit 448f318 into rust-osdev:main Jan 8, 2025
5 checks passed
@mrjbom mrjbom deleted the acpi-Remove-Clone-Copy-traits-for-MADT branch January 8, 2025 17:36
IsaacWoods pushed a commit to IsaacWoods/acpi that referenced this pull request Jan 30, 2025
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