From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Sat, 24 Nov 2012 11:05:46 -0700
Subject: [U-Boot] [PATCH v4 5/7] gpt: Support for GPT (GUID Partition
	Table) restoration
In-Reply-To: <001001cdc7ea$bac41620$304c4260$%wilczek@samsung.com>
References: <1352452938-2375-1-git-send-email-p.wilczek@samsung.com>
	<1352458087-20462-1-git-send-email-p.wilczek@samsung.com>
	<1352458087-20462-2-git-send-email-p.wilczek@samsung.com>
	<50AA93BA.20403@wwwdotorg.org>
	<001001cdc7ea$bac41620$304c4260$%wilczek@samsung.com>
Message-ID: <50B10C7A.4070800@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.