[U-Boot] [PATCH v4 3/6] lib: uuid: add functions to generate UUID version 4

[Przemyslaw Marczak]

Hi,

On 03/26/2014 07:47 PM, Stephen Warren wrote:
> On 03/26/2014 06:00 AM, Przemyslaw Marczak wrote:
>> On 03/25/2014 08:28 PM, Stephen Warren wrote:
>>> On 03/19/2014 11:58 AM, Przemyslaw Marczak wrote:
>>>> This patch adds support to generate UUID (Universally Unique Identifier)
>>>> in version 4 based on RFC4122, which is randomly.
>>>>
>>>> Source: https://www.ietf.org/rfc/rfc4122.txt
>
>>>> diff --git a/lib/uuid.c b/lib/uuid.c
>>>
>>>>    /*
>>>>     * UUID - Universally Unique IDentifier - 128 bits unique number.
>>>>     *        There are 5 versions and one variant of UUID defined by RFC4122
>>>> - *        specification. Depends on version uuid number base on a time,
>>>> - *        host name, MAC address or random data.
>>>> + *        specification. Depends on version uuid number base on:
>>>
>>> I still have no idea what "Depends on version uuid number base on" means.
>>
>> It means that each UUID version "result" depends on different source
>> data, as listed here...
>
> How bout replacing that sentence with:
>
> A UUID contains a set of fields. The set varies depending on the version
> of the UUID, as shown below:
>
Ok, no problem.
>>>> + *        - time, MAC address(v1),
>>>> + *        - user ID(v2),
>>>> + *        - MD5 of name or URL(v3),
>>>> + *        - random data(v4),
>>>> + *        - SHA-1 of name or URL(v5),
>
>>>> + * timestamp - 60-bit: time_low, time_mid, time_hi_and_version
>>>> + * version   - 4 bit (bit 4 through 7 of the time_hi_and_version)
>>>> + * clock seq - 14 bit: clock_seq_hi_and_reserved, clock_seq_low
>>>> + * variant:  - bit 6 and 7 of clock_seq_hi_and_reserved
>>>> + * node      - 48 bit
>>>> + * In this version all fields beside 4 bit version are randomly generated.
>>>> + * source: https://www.ietf.org/rfc/rfc4122.txt
>>>
>>> gen_rand_uuid() doesn't actually honor that format; it creates pure
>>> random data rather than filling in any timestamps, clock sequence
>>> data, etc.
>>
>> Actually, yes but two fields are NOT set randomly, and this is what
>> comment includes:
>> "In this version all fields beside 4 bit version are randomly generated."
>> Moreover the gen_rand_uuid() respects endianess for setting bits,
>> and this could be checked on linux host by "uuid -d uboot_uuid_string"
>> in shell.
>
> While it's true that some fields are set non-randomly, most aren't; you
> really can't claim that e.g. placing random data in the timestamp field
> is a valid timestamp.
>
As I said before, I will remove the "version 4" from the comment head 
because the layout is valid for all uuid versions. So we can take into 
account the meaning of each field only after checking the version of 
uuid first.

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