[U-Boot] [PATCH v4 5/6] cmd:gpt: randomly generate each partition uuid if undefined

[Przemyslaw Marczak <p.marczak@samsung.com>]

Hello Stephen,

On 03/25/2014 08:51 PM, Stephen Warren wrote:
> On 03/19/2014 11:58 AM, Przemyslaw Marczak wrote:
>> Changes:
>> - randomly generate partition uuid if any is undefined and CONFIG_RAND_UUID
>>    is defined
>> - print debug info about set/unset/generated uuid
>> - update doc/README.gpt
>>
>> Update existing code to the new library functions.
>
> The changelog should be below the --- line, and a patch description
> should exist.
>

This is the patch description:) and there is also change log below 
"---", but okay I can make some edits.

> Assuming the comments below are fixed,
> Acked-by: Stephen Warren <swarren@nvidia.com>
>
>> diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c
>
>> -static char extract_env(const char *str, char **env)
>> +static int extract_env(const char *str, char **env)
>>   {
>> +	int ret = -1;
>>   	char *e, *s;
>> -
>> +#ifdef CONFIG_RANDOM_UUID
>> +	char uuid_str[UUID_STR_LEN + 1];
>> +#endif
>>   	if (!str || strlen(str) < 4)
>
> The blank line needs to be after the #endif not before the #ifdef, so
> the variable declarations are separate from the code.
>
Ok.
>>   		return -1;
>>
>> -	if ((strncmp(str, "${", 2) == 0) && (str[strlen(str) - 1] == '}')) {
>> -		s = strdup(str);
>> -		if (s == NULL)
>> -			return -1;
>> -		memset(s + strlen(s) - 1, '\0', 1);
>> -		memmove(s, s + 2, strlen(s) - 1);
>> +	if (!((strncmp(str, "${", 2) == 0) && (str[strlen(str) - 1] == '}')))
>> +		return -1;
>
> Since you're inverting that test, you need to change && to || too.
>
No, because the invertion refers to the result of "if" - not one of 
conditions.
!(cond1 && cond2) is the same as:
(!cond1 || !cond2)
so this change is ok.

>> diff --git a/doc/README.gpt b/doc/README.gpt
>> index 5c133f3..51515c8 100644
>> --- a/doc/README.gpt
>> +++ b/doc/README.gpt
>> @@ -101,7 +101,7 @@ Offset  Size    Description
>>   40      8 B     First usable LBA for partitions (primary partition table last
>>   		LBA + 1)
>>   48      8 B     Last usable LBA (secondary partition table first LBA - 1)
>> -56      16 B    Disk GUID (also referred as UUID on UNIXes)
>> +56      16 B    Disk GUID (also referred as UUID on UNIXes) in big endian
>
> According to your earlier comment, GUIDs have a mix of LE and BE fields,
> so I would simply drop this change and the similar change below. Let
> wikipedia or the comment you added near to top of lib/uuid.c specify the
> details.
>
Actually I think that this is an important info here. The information 
about endianness is also placed in few places in lib/uuid.c

>> @@ -160,6 +160,9 @@ To restore GUID partition table one needs to:
>>      Fields 'name', 'size' and 'uuid' are mandatory for every partition.
>>      The field 'start' is optional.
>>
>> +   option: CONFIG_RANDOM_UUID
>> +   If any partition "uuid" no exists then it is randomly generated.
>
> s/"uuid"/UUID/
>
Ok.

Thanks
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com