
On 11/21/2012 06:18 AM, Piotr Wilczek wrote:
Dear Stephen,
Stephen Warren wrote at Monday, November 19, 2012 9:17 PM: On 11/09/2012 03:48 AM, Piotr Wilczek wrote:
The restoration of GPT table (both primary and secondary) is now possible. Simple GUID generation is supported.
...
- for (i = 0; i < parts; i++) {
/* partition starting lba */
start = partitions[i]->start;
if (start && (offset <= start))
gpt_e[i].starting_lba = cpu_to_le32(start);
else
gpt_e[i].starting_lba = cpu_to_le32(offset);
That seems a little odd. The else branch seems fine when !start, but what about when (start && (offset > start))? Shouldn't that be an error, rather than just ignoring the requested start value?
The idea is that if the user provided start address and the partitions does not overlap, the partition is located at the start address. Otherwise partitions are located next to each other.
But if the user provided a start address, shouldn't it always be honored exactly, or any error generated; silently ignoring a start address when there's an overlap seems wrong. Also, the overlap checking seems a little simplistic; it should really sort the partition array and then walk through checking for overlaps, rather than maintaining a single "highest used offset", since there's no requirement for the physical order of partitions to match the order in the partition table, hence why I asked:
Why can't partitions be out of order? IIRC, the GPT spec allows it.
Oh, so is set_gpt_table() an internal-only function? If so, shouldn't it be static and not in the header file?
It could be used by some other code in future.
Perhaps. It can always be made non-static at that time.