From mboxrd@z Thu Jan  1 00:00:00 1970
From: Przemyslaw Marczak <p.marczak@samsung.com>
Date: Mon, 17 Mar 2014 10:17:17 +0100
Subject: [U-Boot] [PATCH v3 3/3] cmd:gpt: randomly generate each
 partition uuid if undefined
In-Reply-To: <20140314161655.552F838018C@gemini.denx.de>
References: <5ef7cdb8df4fb05c3c371e29d7a61e28e1563a68.1394807506.git.p.marczak@samsung.com>
	<72629ef58556732156fadf26a2a48be85704f224.1393600504.git.p.marczak@samsung.com>
	<e7d9246677ecb9a8c774e9e5f8d31ae3fd53d487.1394807506.git.p.marczak@samsung.com>
	<20140314161655.552F838018C@gemini.denx.de>
Message-ID: <5326BD9D.6090002@samsung.com>
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 03/14/2014 05:16 PM, Wolfgang Denk wrote:
> Dear Przemyslaw Marczak,
>
> In message <e7d9246677ecb9a8c774e9e5f8d31ae3fd53d487.1394807506.git.p.marczak@samsung.com> you wrote:
>> Changes:
>> - randomly generate partition uuid if any is undefined
>> - print info about set/unset/generated uuid
>> - update doc/README.gpt
> ...
>> +	int ret = -1;
>>   	char *e, *s;
>> +	char uuid_str[37];
>
> Should we not rather use a #defined macro here instead of the magic
> number 37 ?
>

Ok, then I create proper header for uuid: "include/uuid.h"

>> -			printf("Environmental '%s' not set\n", str);
>> -			return -1; /* env not set */
>> +			printf("%s ", str);
>> +			gen_rand_uuid_str(uuid_str);
>> +			setenv(s, uuid_str);
>> +
>> +			e = getenv(s);
>> +			if (e) {
>> +				puts("set to random.\n");
>
> Can we keep the "var not set" part, so the user understands why U-Boot
> assigns a random ID here?
>

Ok, I will add this info.

>> +		} else {
>> +			printf("%s get from environment.\n", str);
>
> Make this debug() ?
>
>> +		puts("Writing GPT:\n");
>> +
>> +		ret = gpt_default(blk_dev_desc, argv[4]);
>> +		if (!ret) {
>> +			puts("success!\n");
>> +			return CMD_RET_SUCCESS;
>> +		} else {
>> +			puts("error!\n");
>>   			return CMD_RET_FAILURE;
>
> This code is too verbose - I suggest to turn all these puts() into
> debug().
>
Ok.

>>   	} else {
>>   		return CMD_RET_USAGE;
>>   	}
>
> Please invert the logic so you can bail out early and reduce the
> indentation level.
>
Ok.

> Best regards,
>
> Wolfgang Denk
>
This can be changed to debug(), but I leave print error/success info 
when command finish.

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