Skip to content

add missing functionality in multiboot2-header (finding the header, getting tags) #136

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
May 16, 2023
Merged

add missing functionality in multiboot2-header (finding the header, getting tags) #136

merged 5 commits into from
May 16, 2023

Conversation

YtvwlD
Copy link
Contributor

@YtvwlD YtvwlD commented May 14, 2023

This is the first part split off of #133, as recommended by @phip1611 in #133 (comment):

  • find the header
  • add getter for the tags
  • that module align thingy

The getters are very inspired by the ones in multiboot2.

YtvwlD
YtvwlD commented May 14, 2023
View reviewed changes
@YtvwlD YtvwlD force-pushed the bootloader-header branch 2 times, most recently from 8a28823 to 22d0a17 Compare May 15, 2023 08:58
@YtvwlD
Copy link
Contributor Author

YtvwlD commented May 15, 2023 •
edited
Loading

Hm, I think the parsing story is still a bit weird (pass the file, get the portion that is relevant as a slice, pin that and call from_addr), but it's workable and better than before. :)

I'd really like to just pass the file slice and get a reference to the header, but I don't know if this is workable with the inner references.

Edit: Forget that, the intermediate slice is needed, so that the whole file can be dropped.

@phip1611 phip1611 mentioned this pull request May 15, 2023
Closed
@phip1611
Copy link
Member

Please fix the minor CI problems and then we are good to go :)

@YtvwlD YtvwlD force-pushed the bootloader-header branch from 1cba602 to 8da2fcd Compare May 15, 2023 18:12
@YtvwlD
Copy link
Contributor Author

YtvwlD commented May 15, 2023 •
edited
Loading

Well, that doesn't seem to work. :)

Dereferencing pointers doesn't seem to be const in MSRV.

@YtvwlD YtvwlD force-pushed the bootloader-header branch from 8da2fcd to 55a77e9 Compare May 15, 2023 18:16
@phip1611
Copy link
Member

Oh.

#[allow(clippy::missing_const_for_fn)] is fine - could you add it, please?

YtvwlD added 2 commits May 16, 2023 13:32
@YtvwlD
multiboot2-header: Make find_header return a Result
a190e48 
This should make the function easier to use:
If the header is not properly aligned or cut off, the function
previously just returned None or might have even panicked.
@YtvwlD
multiboot2-header: Make from_addr return a Result
e1b7ffb
@YtvwlD YtvwlD force-pushed the bootloader-header branch from 55a77e9 to e1b7ffb Compare May 16, 2023 11:35
@YtvwlD
Copy link
Contributor Author

YtvwlD commented May 16, 2023

I didn't break that failing lint. :)

@phip1611
Copy link
Member

I didn't break that failing lint. :)

yes, you're right. I'll fix that soon

@phip1611 phip1611 merged commit 3b9c72f into rust-osdev:main May 16, 2023
@phip1611
Copy link
Member

thanks for the contribution!

@YtvwlD YtvwlD deleted the bootloader-header branch May 23, 2023 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phip1611 phip1611 phip1611 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@YtvwlD @phip1611