gpt icon indicating copy to clipboard operation
gpt copied to clipboard

Improve ProtectiveMBR API

Open oldgalileo opened this issue 1 year ago • 1 comments

Currently the API for creating a ProtectiveMBR is somewhat unclear, or at least easy to misuse. While the documentation does explain that the input should be the number of logical blocks not including the PMBR, the naming and much around this is unclear.

Specifically, the lb_size term is overloaded. On GptConfig, you have logical_block_size which expects a gpt::disk::LogicalBlockSize value. lb_size itself is set to be a gpt::disk::LogicalBlockSize in many of the tests and documentation.

let mbr = gpt::mbr::ProtectiveMBR::with_lb_size(<blah>); // Wants size _in_ Logical Blocks, not size _of_ Logical Block

oldgalileo avatar Jul 11 '22 15:07 oldgalileo

I think it makes sense to clear up these functions by moving the lb part of the function name to its signature through the introduction of a LogicalBlockLength type, as a counterpart to the LogicalBlockSize enum.

Maybe even just a Blocks(u32) tuple struct.

For the record, I find the fault here to be more with the EFI spec. The confusion there is real when it comes down to the implementation side of things.

oldgalileo avatar Jul 11 '22 15:07 oldgalileo