[U-Boot] [PATCH v4 5/7] gpt: Support for GPT (GUID Partition Table) restoration

[Piotr Wilczek]

> -----Original Message-----
> From: Stephen Warren [mailto:swarren at wwwdotorg.org]
> Sent: Saturday, November 24, 2012 7:06 PM
> To: Piotr Wilczek
> Cc: u-boot at lists.denx.de; 'Minkyu Kang'; 'Kyungmin Park'; 'Chang Hyun
> Park'; Lukasz Majewski; 'Stephen Warren'; 'Tom Rini'; 'Rob Herring'
> Subject: Re: [PATCH v4 5/7] gpt: Support for GPT (GUID Partition Table)
> restoration
> 
> 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:
> 
You are right. Ignoring start address when there's overlap is a bad idea and
I change the code.

The overlap check method is simple: the offset points to the first address
where the next partition can start. At first it is equal to the first usable
lba, then is increased by size of the previous partition (now I see that
there's a bug in the code I posted, it should be: offset +=
partitions[i]->size);
I think that this simple overlap check method should be enough at the
moment.

> >> 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.

Making it non-static I say that it was designed to be used by other code.