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

[Przemyslaw Marczak]

On 03/26/2014 07:32 PM, Stephen Warren wrote:
> On 03/26/2014 06:01 AM, Przemyslaw Marczak wrote:
>> 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.
>
>>>> 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.
>
> OK. It seems better to get the code right when first introduced, but as
> long as it gets simplified, I guess that's fine.
>
>>>> 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[])
> ...
>>> This duplicates some code; the call to gen_rand_uuid(). I think it would
>>> be better as:
>>>
>>> if (argc < 2)
>>>      return CMD_RET_USAGE;
> ...
>> Yes, this is better, but the first condition should be as:
>> if ((argc != 1) || (argc != 2))
>
> s/||/&&/
>
Ah, my mistake.
This should be as:
if (argc > 2)
	return CMD_RET_USAGE;

since argc value could be only "1" or only "2".

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