From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Mon, 03 Mar 2014 10:47:15 -0700
Subject: [U-Boot] [PATCH 1/2] lib: uuid: add function to generate UUID
 version 4
In-Reply-To: <53148759.5020001@samsung.com>
References: <72629ef58556732156fadf26a2a48be85704f224.1393600504.git.p.marczak@samsung.com>
	<5310BF65.6040603@wwwdotorg.org> <53148759.5020001@samsung.com>
Message-ID: <5314C023.5070505@wwwdotorg.org>
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/03/2014 06:44 AM, Przemyslaw Marczak wrote:
> Hello Stephen,
> Thank you for review.
> 
> On 02/28/2014 05:55 PM, Stephen Warren wrote:
>> On 02/28/2014 08:18 AM, Przemyslaw Marczak wrote:
>>> lib/uuid.c:
>>> Add get_uuid_str() - this function returns 36 character hexadecimal
>>> ASCII
>>> string representation of a 128-bit (16 octets) UUID (Universally Unique
>>> Identifier) version 4 based on RFC4122, which is randomly generated.
>>>
>>> Source: https://www.ietf.org/rfc/rfc4122.txt
>>
>>> diff --git a/disk/part_efi.c b/disk/part_efi.c
>>
>>> @@ -132,9 +113,11 @@ void print_part_efi(block_dev_desc_t * dev_desc)
>>>               le64_to_cpu(gpt_pte[i].ending_lba),
>>>               print_efiname(&gpt_pte[i]));
>>>           printf("\tattrs:\t0x%016llx\n", gpt_pte[i].attributes.raw);
>>> -        uuid_string(gpt_pte[i].partition_type_guid.b, uuid);
>>> +        uuid_bin = (unsigned char *)gpt_pte[i].partition_type_guid.b;
>>> +        uuid_bin_to_str(uuid_bin, uuid);
>>
>> I don't know why you need the uuid_bin temporary variable; you could
>> just as well do the cast as part of the function parameter. Not a big
>> deal though.
>
> Just because the line was too long.

You can wrap the parameter list.

>>> +char *get_uuid_str(void)
>>
>> This function name isn't particularly good; it gives no hint that it's
>> generating a random UUID. Perhaps generate_random_uuid_str() would be
>> better.
> 
> What about this?
> 
> /* To generate bin uuid */
> void gen_rand_uuid(unsigned char *uuid)

I don't think there's a need to shorten the words so much, but at least
that's accurate.