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

simplify find_kernel for x86_64 #376

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

cagatay-y
Copy link
Contributor

The commits are very small to ease review and I think at least some of them can be squashed before merging. If we decide not to squash them all into a single commit, I need to push the commits one-by-one for CI before merging.

In cases where we want to page a range rather than a certain number of
pages, use map_page to eliminate the need to calculate the page count.
containing_page calls already align the addresses down. As such, we do not need to align the addresses down ourselves.
Instead of checking if it is the first iteration of the loop in every iteration, handle the first iteration of the modules iterator outside the loop.
@cagatay-y cagatay-y force-pushed the find-kernel branch 2 times, most recently from c909766 to 420b9e8 Compare October 1, 2024 12:05
for m in modules {
found_module = true;

if start_address == 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed this line is to check if this is the first iteration of the loop (because the variable is initialized to 0) but if it is possible that some of the modules have 0 as their start value, my assumption is incorrect and the new code behaves differently. I don't think that is possible, though.

@mkroening mkroening self-requested a review October 1, 2024 13:20
@mkroening mkroening self-assigned this Oct 1, 2024
@mkroening
Copy link
Member

If you think these commits are best for reviewing, we don't have to squash before merging. That way it will remain easy to review in the future. :)

@cagatay-y cagatay-y force-pushed the find-kernel branch 2 times, most recently from e15f3bf to afa33dc Compare October 3, 2024 11:42
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