From mboxrd@z Thu Jan  1 00:00:00 1970
From: Przemyslaw Marczak <p.marczak@samsung.com>
Date: Thu, 27 Mar 2014 10:17:36 +0100
Subject: [U-Boot] [PATCH v4 4/6] new commands: uuid and guid - generate
 random unique identifier
In-Reply-To: <53331D26.7000201@wwwdotorg.org>
References: <5ef7cdb8df4fb05c3c371e29d7a61e28e1563a68.1394807506.git.p.marczak@samsung.com>
	<1395251911-26540-1-git-send-email-p.marczak@samsung.com>
	<1395251911-26540-4-git-send-email-p.marczak@samsung.com>
	<5331DAF5.7040400@wwwdotorg.org> <5332C17E.7050409@samsung.com>
	<53331D26.7000201@wwwdotorg.org>
Message-ID: <5333ECB0.7080203@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/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