Storing a BootEntry's ID inside it?

[danya02]

I'm currently writing a boot menu-type program, where I choose one of the boot entries, then set BootNext to it. So far, I've managed to find the boot entry I need using the VarManager::get_boot_entries method, however I don't know the BootEntry's ID in order to write it into the BootNext variable. That value isn't stored in a Boot#### variable, but is instead the name of that variable, so it doesn't get parsed from the variable's content.

For now, I have workarounds available, including performing parallel iteration over the get_boot_entries() and get_boot_order(), or getting the boot order list manually from the BootOrder variable. However, I think it would be easier if the ID were part of a BootEntry.

[iTrooz]

Hey !

I presume you were using v1.4.0. Since then, I fixed the issue in main by making get_boot_entries() return a tuple of (Result<BootEntry>, Variable) (See https://github.com/iTrooz/efiboot-rs/blob/d81047c2bfe507102966dc5d5619036e6a4f29f8/efiboot/src/cli/boot/get_entries.rs for usage)

Please note that the current API on the main branch is heavily in beta. I reserve myself the right to do breaking changes at any moment, until I make a 2.0 release, at which point the API will be stable. Until then, there should be no new release.

Its ok if you want to use it, but be ready to modify your code a bit when updating

[danya02]

Thanks for your reply. I think I'll stick to the released version for now, because I don't know when I'll be coming back to this project, and I am able to work around the issue. I might reconsider if I encounter something that can't be worked around that I need.

Though I did notice one such thing, and it is that some of the user-visible types, like the BootEntry, or FilePathList, don't have Debug implementations, which you might want if you're using this as a library like I am. You probably also fixed that already as well, though.

Though, if you're going to be changing APIs for 2.0, then you may still consider embedding the boot ID into the record. One thing I was confused about is how in v1.4 you were parsing the u16 boot IDs only to then put them into a string instead of exposing the sequence of u16s like you do now -- doing that didn't make a sense to me, because by embedding the numbers into the string you now have to parse them back out of the string, adding a place for errors where there shouldn't be.

But then, in your linked sample, you end up needing to parse the number back out of the string, because it's being thrown away at some point before. That except will never trigger, but its presence is making it seem like it could, which is the downside of passing strings everywhere.

So, my recommendation for v2.0 (or maybe v3.0 as it's a fairly involved thing) is to put all the (important) EFI-standard variable names into their own structs. Then, you would be able to return a BootNumberVariable { boot_id: u16 } from that iterator, instead of a generic Variable with opaque string names.

[iTrooz]

Though, if you're going to be changing APIs for 2.0, then you may still consider embedding the boot ID into the record.

I don't think I'll do this, because I want to have one type dedicated to the variable content, with no relation to where it comes from. That said, I could imagine a type which combines the variable's identifier, and its content. I don't see the need for it, rather than a tuple like I'm doing now, though.

But then, in your linked sample, you end up needing to parse the number back out of the string, because it's being thrown away at some point before. That except will never trigger, but its presence is making it seem like it could, which is the downside of passing strings everywhere.

Indeed, this is one of the things which may change before 2.0

So, my recommendation for v2.0 (or maybe v3.0 as it's a fairly involved thing) is to put all the (important) EFI-standard variable names into their own structs. Then, you would be able to return a BootNumberVariable { boot_id: u16 } from that iterator, instead of a generic Variable with opaque string names.

I think this is a good idea ! Specifically, I could make a BootVariable type, which would be a "specialisation" of the Variable type, and which implements .into::<Variable>()

In general, I don't have a lot of experience building APIs, so your feedback is really interesting ! Thank you !

[danya02]

I want to have one type dedicated to the variable content, with no relation to where it comes from

How about an enum?

/// Different categories of variables to choose from
#[derive(derive_more::From, Debug, Clone)]
pub enum VariableName {
  Efi(EfiStandardVariableName),
  Vendored(VendoredVariableName),
}

/// Vendor-specific variables are opaque
#[derive(Debug, Clone)]
pub struct VendoredVariableName {
  pub uuid: ...,
  pub name: ...String...,
}

/// EFI Standard variables are either the ones we care about explicitly,
/// or just the ones that have that vendor UUID
#[derive(derive_more::From, Debug, Clone)]
pub enum EfiStandardVariableName {
  Specific(SpecificEfiVariableName),
  Generic(...String...),
}

/// This is where we list all the variable names that are semantically meaningful to our app
#[derive(derive_more::From, Debug, Clone, Copy)]
#[non_exhaustive]
pub enum SpecificEfiVariableName {
  BootOrder,
  BootEntry(BootEntryVariableName),
  // ...
}

/// After cascading through a few of the enums, we finally get to this struct,
/// which is unambiguous in its meaning.
/// It can even be made Copy at this point, because it's just a number
#[derive(PartialEq, Debug, Clone, Copy)]
pub struct BootEntryVariableName {
  pub boot_id: u16, 
}

Once you have this, you can add a cascade of parse implementations, and the presence of the generic options ensures that whatever string you put in, you will get an instance of VariableName.

impl VariableName {
  pub fn new(uuid: ..., name: ...String...) -> Self {
    if uuid == MAGIC_EFI_VENDOR_UUID {
      EfiStandardVariableName::new(name).into()
    } else {
      VendoredVariableName { uuid, name }.into()
    }
  }
}

impl EfiStandardVariableName {
  pub fn new(name: ...String...) -> Self {
    match SpecificEfiVariableName::try_from_name(name) {
      Some(specific) => specific.into(),
      None => Self::Generic(name),
    }
  }
}

impl SpecificEfiVariableName {
  pub fn try_from_name(name: ...String...) -> Option<Self> {
    if name == "BootOrder" {Some(Self::BootOrder)}
    else if let Some(boot_id) = parse_boot_entry_id_from_bootXXXX(name) {Some(BootEntryVariableName {boot_id}.into())}
     else {None}
   }
}

This all looks scarily verbose, but it's the kind of code you write once and then never touch it again. Also, you can reduce the amount of code being written with clever use of macros: for example, derive_more, which I've mentioned here, will generate the From implementations for your enums automagically.

[iTrooz]

Hey ! Sorry about the long delay

For now I'm not in the mood to think about/discuss Rust architecture stuff (I'm currently handling another project), but I'll check this out in detail when I come back to this project

Thanks !

[iTrooz]

Hey ! Finally got time to properly review your suggestion

So, for now I don't think if is useful to add this whole enum system, because the useful case I see for it is a boot entry having access to its boot id. So for now, I added a BootVariable type which is composed of the Boot Entry (purely the parsed content of the variable), and a boot id.

If I see the need for more "special" variable types like this in the future, I'll add the enum you suggested to help

I'm closing this issue since the initial problem is solved

[iTrooz]

By the way, is this project open source ?

I'm currently writing a boot menu-type program, where I choose one of the boot entries, then set BootNext to it

I'd love to take a look at it, and potentially contribute :)