[U-Boot] [PATCH v4 4/6] new commands: uuid and guid - generate random unique identifier

[Przemyslaw Marczak]

Hello Stephen,

On 03/25/2014 08:37 PM, Stephen Warren wrote:
> On 03/19/2014 11:58 AM, Przemyslaw Marczak wrote:
>> Those commands basis on implementation of random UUID generator version 4
>> which is described in RFC4122. The same algorithm is used for generation
>> both ids but string representation is different as below.
>>
>> char:  0        9    14   19   24         36
>>         xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
>> UUID:     be     be   be   be       be
>> GUID:     le     le   le   be       be
>>
>> Commands usage:
>> - uuid <varname(optional)>
>> - guid <varname(optional)>
>
> Square brackets are usually used to indicate optional parameters:
>
> - uuid [<varname>]
> - guid [<varname>]
>

Ok, I change this.

>> diff --git a/include/common.h b/include/common.h
>
>>   #if defined(CONFIG_RANDOM_MACADDR) || \
>>   	defined(CONFIG_BOOTP_RANDOM_DELAY) || \
>>   	defined(CONFIG_CMD_LINK_LOCAL) || \
>> -	defined(CONFIG_RANDOM_UUID)
>> +	defined(CONFIG_RANDOM_UUID) || \
>> +	defined(CONFIG_CMD_UUID)
>
> Why not require that if you want to use CONFIG_CMD_UUID, you must define
> CONFIG_RANDOM_UUID too? You can even make that automatic in
> include/config_fallbacks.h which already does similar things:
>
> #if defined(CONFIG_CMD_FAT) && !defined(CONFIG_FS_FAT)
> #define CONFIG_FS_FAT
> #endif
>
> That way, you won't need to touch lib/Makefile in this patch either, or
> modify the ifdef that wraps gen_rand_uuid().
>

I change this part of code in one of my other patch set which can be 
found here: http://patchwork.ozlabs.org/patch/332499/
After apply those changes then I add some automation here.

>> diff --git a/lib/uuid.c b/lib/uuid.c
>
>
>> +#ifdef CONFIG_CMD_UUID
>> +int do_uuid(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> +{
>> +	char uuid[UUID_STR_LEN + 1];
>> +	uuid_str_t str_format;
>> +
>> +	if (!strcmp(argv[0], "uuid"))
>> +		str_format = UUID_STR_FORMAT_STD;
>> +	else
>> +		str_format = UUID_STR_FORMAT_GUID;
>> +
>> +	if (argc == 1) {
>> +		gen_rand_uuid_str(uuid, str_format);
>> +		printf("%s\n", uuid);
>> +	} else if (argc == 2) {
>> +		gen_rand_uuid_str(uuid, str_format);
>> +		setenv(argv[1], uuid);
>> +	} else {
>> +		return CMD_RET_USAGE;
>> +	}
>
> This duplicates some code; the call to gen_rand_uuid(). I think it would
> be better as:
>
> if (argc < 2)
> 	return CMD_RET_USAGE;
> gen_rand_uuid_str(uuid, str_format);
> if (argc == 1)
> 	printf("%s\n", uuid);
> else
> 	setenv(argv[1], uuid);
>

Yes, this is better, but the first condition should be as:
if ((argc != 1) || (argc != 2))

>> +U_BOOT_CMD(uuid, CONFIG_SYS_MAXARGS, 1, do_uuid,
>> +	"UUID - generate Universally Unique Identifier version 4",
>
> Would it be batter to say "a random ..." rather than "... version 4"?
> I'm not sure if the details of the version matter so long as its a valid
> UUID, and certainly the fact the generated UUID is random is likely more
> interesting.
>
>> +	"<varname(optional)>\n"
>
> 	"[<varname>]\n"
>
>
Ok, I also apply those two commands above.

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