Skip to content

Add handle_protocol_mut #19

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add handle_protocol_mut #19

wants to merge 9 commits into from

Conversation

csssuf
Copy link
Owner

@csssuf csssuf commented Aug 11, 2017

Some applications may need to acquire a mutable protocol from handle_protocol.

Note: includes commits also in #18

csssuf added 9 commits August 4, 2017 15:43
This is immediately useful for checking partition types/GUIDs for block
and disk I/O, but is also useful elsewhere.
Some function calls require image handles in place of the
LoadedImageProtocol struct, so track it for library consumers that may
want to use it.
Some applications want to set or read load_options (and the
corresponding size field).
Implement handle_protocol_mut, and stop using mem::transmute in
handle_protocol.
@csssuf csssuf requested a review from bgilbert August 11, 2017 01:45
@bgilbert
Copy link

Would it make sense to implement handle_protocol as a wrapper around handle_protocol_mut?

@csssuf
Copy link
Owner Author

csssuf commented Aug 11, 2017

I don't believe so; typically Rust APIs that expose both get_ref and get_mut-like functions don't implement get_ref by downcasting the result of get_mut. It might make sense to factor out the actual UEFI call into an internal helper function though, if you think that would be better.

Copy link

@bgilbert bgilbert left a comment

Choose a reason for hiding this comment

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

Last two commits LGTM as is.

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