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

Reduce memory allocations and simplify the mbmem loop #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Schwarzemann
Copy link

The collect_multiboot_info function allocates memory for mmap using malloc. Depending on the size of the multiboot_mmap array, this could be a lot of memory. Instead, a stack-allocated array or a pre-allocated buffer to store the mmap data would work perfectly and the mbmem loop that converts the mmap records is somewhat complex and difficult to follow.

The collect_multiboot_info function allocates memory for mmap using malloc. Depending on the size of the multiboot_mmap array, this could be a lot of memory. Instead, a stack-allocated array or a pre-allocated buffer to store the mmap data would work perfectly and the mbmem loop that converts the mmap records is somewhat complex and difficult to follow.
@mcayland
Copy link
Collaborator

Thanks for the patch! Whilst the repositories are hosted on github, the project still makes use of mailing lists for patches/code review. If you can resend the patch to the OpenBIOS mailing list (https://mail.coreboot.org/postorius/lists/openbios.openbios.org/) we should be able to get this reviewed fairly quickly.

One question: do you have a working test case for amd64? I've got tests for all the other archs, but it would give me more confidence when reviewing the patch if I have a way to functionally verify it.

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.

None yet

2 participants