gpt icon indicating copy to clipboard operation
gpt copied to clipboard

Support custom partition GUIDs

Open LucaFulchir opened this issue 3 years ago • 5 comments

Provides:
Support to specify and read partition UUID that are not hardcoded in the library

Motivation:
I am writing a tool that needs to handle gpt in the most general way possible, I can't really limit myself to a few partition types.

How:
The problem is partition_type::Type, which contains pub guid: &'static str,.
This can't support anything non-hardcoded. The solution is either to keep trying to use &str and play around with lifetimes and hide allocations somewhere in ugly ways, or just use String

At that point the macro is modified so that instead of generating const NAME : Type it generates fn NAME() -> Type which allocates the string for the guid on the heap.

The macro will return a Type as before, but when called with an unknown UUID it will set the os to Unknown, and will return error only if it receives a string that is not a uuid or a valid name

Is this good enough or do you prefer other approaches?

TODO:
before merging:

  • tests pass, compiles and I think it's faily safe, but I have NOT actually tested the result (need more time, meanwhile please comment)
  • the signature of partition_type::Type has changed, therefore this needs a semversion bump (not included)

P.S.: can I suggest also enabling rustfmt in your editor of choice?

LucaFulchir avatar Jan 11 '22 14:01 LucaFulchir

Wouldn't it make more sense to make the guid, a uuid::UUID? Then you never have any allocation and can keep the constants. https://github.com/Quyzi/gpt/blob/9b6d725106ddd8962a4a8b1bb192455b841a9883/src/macros.rs#L29-L32 When 1.0 of uuid get's released it will be possible to parse a uuid string in the const context see: https://docs.rs/uuid/1.0.0-alpha.1/uuid/struct.Uuid.html#method.try_parse. This probably even improve the current lookup code: https://github.com/Quyzi/gpt/blob/9b6d725106ddd8962a4a8b1bb192455b841a9883/src/partition_types.rs#L109-L113

soerenmeier avatar Mar 29 '22 20:03 soerenmeier

I assumed there was a reason for the string aside from the constness, and didn't want to change that too much.

It's fairly easy to change to uuid, but it seems you want to wait for the 1.0 stable version of uuid, so I'll revisit this PR once it's released

LucaFulchir avatar Apr 09 '22 09:04 LucaFulchir

...changed my mind and I had time.
Using Uuid now, and I changed the dependency of uuid to 1.0.0-alpha.1

It should be practically ready for when they merge 1.0

LucaFulchir avatar Apr 09 '22 13:04 LucaFulchir

uuid 1.0 is out, I updated the PR.

LucaFulchir avatar Apr 22 '22 18:04 LucaFulchir

Just for what it is worth, this can be done today by making the struct by hand:

    let partition_esp = Type {
        guid: "C12A7328-F81F-11D2-BA4B-00A0C93EC93B",
        os: OperatingSystem::Custom("EFI System Partition".into()),
    };

grahamc avatar Jun 12 '22 10:06 grahamc

Thanks @LucaFulchir

soerenmeier avatar Apr 02 '23 15:04 soerenmeier