Skip to content

doc/uefi: improve Protocol and Device Path documentation #1612

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 6 commits into
base: main
Choose a base branch
from

Conversation

phip1611
Copy link
Member

@phip1611 phip1611 commented Apr 7, 2025

Make the role of the high-level DevicePath wrapper more clear.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@phip1611 phip1611 requested a review from nicholasbishop April 7, 2025 12:51
@phip1611 phip1611 self-assigned this Apr 7, 2025
@phip1611
Copy link
Member Author

phip1611 commented Apr 7, 2025

Hey @seijikun,
as you were working a lot with device paths recently; do you think we need more improvements to the documentation? Do you think these improvements here are sufficient?

@phip1611 phip1611 mentioned this pull request Apr 7, 2025
13 tasks
@seijikun
Copy link
Contributor

seijikun commented Apr 7, 2025

I don't remember in full detail, but:

One thing I happend to "stumble" over while developing was, implementing the scsi path_node() method (PR pending).
The raw API returned me a *DevicePathProtocol and if I remember correctly, I tried to cast that to a &DevicePathNode, which just hard-crashed (no panic, no result, just .. gone) the application at some point later. In the end, it turned out to be crashing somewhere here:

    fn deref(&self) -> &Self::Target {
        unsafe { DevicePathNode::from_ffi_ptr(self.0.as_ptr().as_ptr().cast()) }
    }

So it took me a while to understand how the API was meant to be used and what the relationship between FFiDevicePath, DevicePathProtocol and DevicePath/DevicePathNode is. Admitedly .. it's still rather hazy :D

But I think that's mostly interna - so not very interesting for users of uefi, and more for potential contributors.

Personally, I mostly use to_string() and that's easily discoverable 👌

@phip1611 phip1611 removed the request for review from nicholasbishop April 7, 2025 15:21
@phip1611 phip1611 marked this pull request as draft April 7, 2025 15:21
@phip1611 phip1611 marked this pull request as ready for review April 8, 2025 07:30
@phip1611 phip1611 changed the title doc/uefi: improve device path documentation doc/uefi: improve Protocol and Device Path documentation Apr 8, 2025
@phip1611 phip1611 requested a review from nicholasbishop April 8, 2025 07:30
@phip1611 phip1611 force-pushed the doc-devicepath branch 2 times, most recently from 3213ebd to 999927a Compare April 8, 2025 07:34
//! another.
//!
//! Implementation-wise, a protocol is a `C` struct holding function pointers
//! and/or data. Please note that some protocols may use [`core::ptr::null`] as
Copy link
Member Author

Choose a reason for hiding this comment

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

@nicholasbishop I'd like to add more about protocol interfaces being null; why can they be null and when is this valid? Can you help?

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