PXE: Fix BaseCode::discover optional argument

[nsemmel]

NOTE: This commit changes the API for BaseCode::discover, but should continue to be source-compatible. Edit: DiscoverInfo will break in cases where the generic can't be inferred, or where it's explicitly declared (eg, DiscoverInfo<[Server; 4]> needs to be changed to DiscoverInfo<4>).

The info parameter in BaseCode::discover is an optional pointer argument (it may be NULL), but is incorrectly specified in the Rust extern definition as Option<*const T>. This type is (at least) one byte larger than a Pointer on all platforms, making it incompatible with the Optional Pointers in EFI. Two possible fixes:

1. Remove Option wrapper (this is what the PR contains)
2. Convert to Option<&T> (chose not to use this since converting references requires unsafe)

The DiscoverInfo struct had odd bounds associated with it, and being generic over T: ?Sized doesn't make sense when the server list has to be a fixed size. This could be removed or broken out into a separate commit if needed.

Checklist

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

[nsemmel]

A quick grep (grep -nIr 'Option<\*') shows that this was the only pointer type wrapped in an Option, so it looks like this is just a one-off thing.

[nicholasbishop]

Thanks for the PR! Good catch on the bad Option type there. I've left a suggestion to use a DST struct instead of generics.

[nicholasbishop]

Could we change this to a DST type instead? That's how we've handled similar things elsewhere in this crate, see for example PcrEvent, and how we convert from a thin pointer to the DST type in PcrEvent::from_ptr. That way we don't need to know what N is at compile time.

[nsemmel]

Looking at this closer, I don't think this can be a DST - or at least I'm not sure how to go about turning it into one. Unlike PcrEvent, this type needs to be constructed from Rust and passed across the FFI boundary - I'm not aware of a way to create a new DST instance with relying on dynamic allocation.

[nicholasbishop]

Ah sorry I should have provided an example of that. The way to do it without allocation is to provide a byte buffer. Here's a not-yet-merged example for PcrEvent: https://github.com/rust-osdev/uefi-rs/pull/631/files#diff-3c3ebe7fc6a06f58897b24d5e0420ad545037b8cb91dae43d5551037122d19cfR136

[nsemmel]

Updated. I left it as a separate commit for comparison, can be squashed.

[nsemmel]

Should I include the #[must_use] attributes that it's complaining about in the lints? I don't think they're appropriate here.

[nicholasbishop]

Should I include the #[must_use] attributes that it's complaining about in the lints? I don't think they're appropriate here.

Yes, please add must_use there. The idea is that if you call a function that doesn't do anything except, say, return a bool, then not using the return value is likely a bug.

[nsemmel]

Sorry - that should be all the lints fixed. For some reason I had it in my head that clippy would yell about rustfmt issues as well.

[nicholasbishop]

Lgtm, thanks again for the PR :)