multiboot2: Allow to get Custom Tags

[phip1611]

This is my first draft to implement #116, i.e., to parse custom Multiboot2 tags.

This PR allows getting custom tags from the Multiboot2 boot information.

The PR should be reviewed commit by commit. May I ask @IsaacWoods for feedback and @blitz for your opinion about the new API?

TL;DR usage:

    #[repr(C, align(8))]
    struct CustomTag {
        // new type from the lib: has repr(u32)
        tag: SpecifiedOrCustomTagTypeSerialized,
        size: u32,
        // begin of inline string
        name: u8,
    }

    let tag = multiboot2_boot_information
        // this function is now public!
        .get_tag(0x1337)
        .unwrap()
        // type definition from end user. must be Sized!
        .cast_tag::<CustomTag>();
    let name = &tag.name as *const u8 as *const c_char;
    let str = unsafe { CStr::from_ptr(name).to_str().unwrap() };
    assert_eq!(str, "name");

Of course, using custom tags, brings all the pain of doing raw pointer magic to the library users.

[IsaacWoods]

Looks good! Some comments about a potential ergonomics improvement, and also some names.

[IsaacWoods]

This and SpecifiedOrCustomTagTypeSerialized are quite long names. If you're alright with the breaking change, I think this being TagType and the current type being renamed to StandardTagType could work? Not sure what a better word for the Serialized bit would be but it also doesn't quite fit.

[phip1611]

I have an idea how I can simplify everything and have cleaner names. Let's see how it works out in the end.

[blitz]

Cleaner (shorter) names would really help. I can't help but gloss over the name after the third word. ;)

[blitz]

Worst offender: SpecifiedOrCustomTagTypeSerialized :-D

[phip1611]

I hope you like the new API more. Now there is only TagType and TagTypeId.

[blitz]

Thanks for working on this!

Regarding the naming:

• "specified" could be "Standard"
• "custom" could be "NonStandard" or "VendorSpecific"

[blitz]

Regarding the API, for your example of a tag that contains a string it could look like this instead:

#[repr(C, align(8))]
struct CustomTag {
  name: [u8], // flexible array
}

impl Tag for CustomTag {
  pub const ID: u32 = 0x1337;
}

For the user the pointer handling then completely disappears. It would also take care of having to handle tag IDs manually. The only downside is that CustomTag can only be constructed with unsafe, but this would be hidden in the library.

As a user I would then like to do use something like find_tag::<CustomTag>() -> Option<CustomTag>.

[phip1611]

Regarding the API, for your example of a tag that contains a string it could look like this instead:

#[repr(C, align(8))]
struct CustomTag {
  name: [u8], // flexible array
}

impl Tag for CustomTag {
  pub const ID: u32 = 0x1337;
}

Impossible, as this is not Sized. I didn't find a way to enable the compilation ?Sized structs. https://doc.rust-lang.org/error_codes/E0607.html?highlight=fat%20pointer#

@blitz

I think, ?Sized structs are incompatible on the ABI level.

[blitz]

Impossible, as this is not Sized. I didn't find a way to enable the compilation ?Sized structs. https://doc.rust-lang.org/error_codes/E0607.html?highlight=fat%20pointer#

You may have to cast through a slice to get the required size information.

I think, ?Sized structs are incompatible on the ABI level.

Are you sure? The way I understand it is that references to the ?Sized struct become fat pointers (pointers + size) and the struct itself looks like in C. This would be ok. I've verified this using objdump and it looks good.

[phip1611]

As a user I would then like to do use something like find_tag::<CustomTag>() -> Option<CustomTag>.

It feels unnecessarily overengineered to me. I'd stick with mbi.get_tag(CustomTag::ID) as it also aligns with the existing code base.

[phip1611]

If no further comments are made, I'm going to merge this in 7 days.