uefi-raw: unified boolean type

[phip1611]

This streamlines the existing usages of u8 and bool. Note that BootPolicy from #1297 is seamlessly integrated into u8 and bool.

Steps to Undraft

• close LoadFileProtocol and LoadFile2Protocol #1297 and rebase onto
• reevaluate the discussion from uefi: add BootPolicy type #1326 (comment)

Checklist

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

[nicholasbishop]

bool is an interesting case in terms of how far to go with worrying about undefined behavior. Creating a bool from any value other than 0 or 1 is UB. (Miri correctly catches it: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=8777a1510372038e487aea18979f78ac.) The UEFI spec says pretty much the same thing: 0 and 1 are valid, other values are undefined. However, in C code it's not actually that hard to accidentally end up with a boolean-ish value that isn't 0 or 1. For example: bitwise operators like flags & MY_FLAG -- in Rust you have to explictly convert that to a bool with (flags & MY_FLAG) != 0, but not so in C.

So this ends up being similar to enums, where we carefully construct wrappers with newtype_enum! -- they can hold any bit pattern, unlike normal enums. If we want to guard against UB, then the Boolean type should be a repr(transparent) wrapper around u8, rather than a Rust bool. I think it's definitely questionable whether it's worth worrying about, but that's the explanation for why I used u8 rather than bool in most places in uefi-raw.

[phip1611]

I've come up with a new design. What do you think?

Open Question:

• We could loosen the requirements. 0bxxxxxx1 can be a a valid bit pattern mapping to true. currently, this is a failure. What do you think?
• Regarding check-raw and the failing CI: I'm not sure if cargo xtask check-raw brings us more benefits than breaks development 🤔 Can you please elaborate what's the idea here, @nicholasbishop ?

[nicholasbishop]

I think this design looks good.

We could loosen the requirements. 0bxxxxxx1 can be a a valid bit pattern mapping to true. currently, this is a failure. What do you think?

Yes, I think loosening the requirements so that any non-zero value is treated as true sounds good. This matches C behavior and is also what r-efi does: https://docs.rs/r-efi/latest/src/r_efi/base.rs.html#488.

Regarding check-raw and the failing CI: I'm not sure if cargo xtask check-raw brings us more benefits than breaks development

I feel that it does. For example, with the field pub check that this PR is failing on, almost all structs in uefi-raw should have all of their fields pub, because otherwise the struct cannot really be constructed outside the crate. It's very easy to forget while writing code, and easy to miss in code review.

In this particular case, I think the easiest thing to do is to change the definition to pub struct Boolean(pub u8). There's no harm in making the field public in uefi-raw, and the Boolean type isn't used in the public API of uefi.