Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork175
uefi: add BootPolicy type#1326
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This type is used in three functions of the UEFI spec. Having an enum instead of aboolean simplifies the interface as the variants can be properly documented.
- public modules- private modules- public uses- private usesThis is consistent with other parts of the code.
This adds the new `BootPolicy` type into `LoadImageSource`.
The existing code duplication is just temporary until the old APIis deleted. Nevertheless, it is nicer to have this conversion closeto the actual type.
| /// various UEFI functions that load files (typically UEFI images). | ||
| /// | ||
| /// This type is not ABI compatible. On the ABI level, this is an UEFI | ||
| /// boolean. |
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.
It seems like it could be made compatible: addrepr(u8) andBootSelection = 1,ExactMatch = 0.
(As long as this type is only passed into UEFI APIs, and not read from them; in that casenewtype_enum might be better.)
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.
Do we want to postpone that decision/discussion to#1307?
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.
As long as the type is only being passed into the API, UB shouldn't be a concern -- we'll always initializeBootPolicy correctly.
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.
Were you just adding thoughts to the discussion or do you want this doccomment to be removed before we merge it? That's not clear to me right now :)
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.
I was thinking we should make changes before merging, but actually since there is no declaredrepr right now, it wouldn't be a breaking change to addrepr(u8) in the future I think. So let's go ahead and merge.
59790f8Uh oh!
There was an error while loading.Please reload this page.
This was decoupled from#1297 for better review-ability.
Checklist