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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
