-
-
Notifications
You must be signed in to change notification settings - Fork 170
Add PXE Base Code protocol #417
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
Conversation
I'm not sure if the failed CI run is caused by the changes made in this pr, I get the same error on the main branch on my machine. |
Thanks for the PR! I am also seeing the i686 compilation failure, filed #418 to fix that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good overall! I've left a few suggestions here and there.
@@ -0,0 +1,116 @@ | |||
use uefi::{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No substantive comment, just wanted to mention I appreciate the nice test here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm -- I'll leave to @GabrielMajeri to merge in case he has any additional comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few more minor changes I propose looking into, and then we should be ready to merge 😄
Would it be possible to publish a new version? I have a pr for https://github.com/rust-osdev/bootloader that depends on this. |
@Freax13 Sure! Will work on making a new release as soon as possible. |
@Freax13 |
This pr adds the PXE Base Code protocol.
I only need
BaseCode::tftp_get_file_size
andBaseCode::tftp_read_file
, but felt weird about only adding a small part of the protocol, so added the other functions as well.I was able to test
BaseCode::tftp_get_file_size
andBaseCode::tftp_read_file
on real hardware.This pr slightly overlaps with #306: Both prs add structures for
EFI_MAC_ADDRESS
andEFI_IP_ADDRESS
.