diff mbox

[U-Boot,V2,2/3] lib: uuid: add functions to generate UUID version 4

Message ID cc0f558724a4d3ea3497b84601038f5f18f37a7b.1394037321.git.p.marczak@samsung.com
State Changes Requested
Delegated to: Tom Rini
Headers show

Commit Message

Przemyslaw Marczak March 5, 2014, 4:45 p.m. UTC
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

Changes:
- add new config: CONFIG_RANDOM_UUID: compile uuid.c and rand.c

Update files:
- include/common.h
- lib/Makefile
- lib/uuid.c

lib/uuid.c:
- add gen_rand_uuid() - this function writes 16 bytes len binary representation
  UUID v4 to address given by user.

- add gen_rand_uuid_str() - this function writes 37 bytes len hexadecimal
  ASCII string representation of 16 bytes binary UUID to address given by user.

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
cc: Stephen Warren <swarren@nvidia.com>
cc: trini@ti.com

---
Changes v2:
- put uuid generation changes in a separate commit
- get_uuid_str() - change name to gen_rand_uuid_str()
- add new function: gen_rand_uuid()
- remove unnecessary '\0' at the end of uuid string
- drop unnecessary error checking
- functions now takes pointers to allocated memory instead of alloc it itself
- add new config option: CONFIG_RANDOM_UUID
---
 include/common.h |    5 +++-
 lib/Makefile     |    4 ++-
 lib/uuid.c       |   81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+), 2 deletions(-)

Comments

Stephen Warren March 10, 2014, 5:37 p.m. UTC | #1
On 03/05/2014 09:45 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
> 
> Changes:
> - add new config: CONFIG_RANDOM_UUID: compile uuid.c and rand.c
> 
> Update files:
> - include/common.h
> - lib/Makefile
> - lib/uuid.c

The patch already contains the list of changed files; there's little
point duplicating this in the patch description.

> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> cc: Stephen Warren <swarren@nvidia.com>
> cc: trini@ti.com

s/cc/Cc/

> diff --git a/lib/Makefile b/lib/Makefile
> index 70962b2..64a430f 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -59,10 +59,12 @@ obj-y += linux_string.o
>  obj-$(CONFIG_REGEX) += slre.o
>  obj-y += string.o
>  obj-y += time.o
> +obj-y += vsprintf.o

Moving vsprintf.o seems entirely unrelated to this patch. If you want to
clean that up, it should be a separate patch.

>  obj-$(CONFIG_TRACE) += trace.o
>  obj-$(CONFIG_BOOTP_PXE) += uuid.o
>  obj-$(CONFIG_PARTITION_UUIDS) += uuid.o
> -obj-y += vsprintf.o
> +obj-$(CONFIG_RANDOM_UUID) += uuid.o
> +obj-$(CONFIG_RANDOM_UUID) += rand.o
>  obj-$(CONFIG_RANDOM_MACADDR) += rand.o

I really hope listing the same object multiple times in obj-y is OK.

Why not sort the lines you added so based on the config variable, so

>  obj-$(CONFIG_RANDOM_MACADDR) += rand.o
> +obj-$(CONFIG_RANDOM_UUID) += rand.o

rather than:

> +obj-$(CONFIG_RANDOM_UUID) += rand.o
>  obj-$(CONFIG_RANDOM_MACADDR) += rand.o

> diff --git a/lib/uuid.c b/lib/uuid.c
> index 803bdcd..c0218ba 100644
> --- a/lib/uuid.c
> +++ b/lib/uuid.c
> @@ -7,6 +7,29 @@
>  #include <linux/ctype.h>
>  #include <errno.h>
>  #include <common.h>
> +#include <part_efi.h>
> +#include <malloc.h>
> +
> +#define UUID_STR_LEN			36
> +#define UUID_STR_BYTE_LEN		37

If UUID_STR_BYTE_LEN is the length in bytes, what units is UUID_STR_LEN
in? I suppose the difference is the NULL terminator, but the names don't
make that clear at all. I would suggest not defining UUID_STR_BYTE_LEN
at all, but rather just writing "+ 1" at the appropriate place in the
source code.

> +#define UUID_BIN_BYTE_LEN		16

Also, s/_BYTE// there.

> +#define UUID_VERSION_CLEAR_BITS		0x0fff

s/CLEAR_BITS/MASK/

> +struct uuid {
> +	unsigned int time_low;
> +	unsigned short time_mid;
> +	unsigned short time_hi_and_version;
> +	unsigned char clock_seq_hi_and_reserved;
> +	unsigned char clock_seq_low;
> +	unsigned char node[6];
> +};

Is that structure definition endianness-safe?

> +/*
> + * gen_rand_uuid() - this function generates 16 bytes len UUID V4 (randomly)
> + *                   and stores it at a given pointer.

I think "this function generates a random binary UUID v4, and stores it
into the memory pointed at by the supplied pointer, which must be 16
bytes in size" would be better.

> +void gen_rand_uuid(unsigned char *uuid_bin)
> +{
> +	struct uuid *uuid = (struct uuid *)uuid_bin;
> +	unsigned int *ptr = (unsigned int *)uuid_bin;
> +	int i;
> +
> +	if (!uuid_bin)
> +		return;

I think you should either (a) assume NULL will never be passed to this
function, or (b) return an error code if it happens. Silently failing to
do anything doesn't make sense.

> +	memset(uuid_bin, 0x0, sizeof(struct uuid));
> +
> +	/* Set all fields randomly */
> +	for (i = 0; i < sizeof(struct uuid) / sizeof(*ptr); i++)
> +		*(ptr + i) = rand();

If the entire thing is filled randomly, why memset() the struct?


> +/*
> + * gen_rand_uuid_str() - this function generates UUID v4 (randomly)
> + * into 36 character hexadecimal ASCII string representation of a 128-bit
> + * (16 octets) UUID (Universally Unique Identifier) version 4 based on RFC4122
> + * and stores it at a given pointer.

There's a lot of duplication in that description. I think "this function
generates a random string-formatted UUID v4, and stores it into the
memory pointed at by the supplied pointer, which must be 37 bytes in
size" would be better.

> +void gen_rand_uuid_str(char *uuid_str)
> +{
> +	unsigned char uuid_bin[UUID_BIN_BYTE_LEN];
> +
> +	if (!uuid_str)
> +		return;

Again, I would drop that error-checking.
Przemyslaw Marczak March 13, 2014, 6:10 p.m. UTC | #2
Hello again,

On 03/10/2014 06:37 PM, Stephen Warren wrote:
> On 03/05/2014 09:45 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
>>
>> Changes:
>> - add new config: CONFIG_RANDOM_UUID: compile uuid.c and rand.c
>>
>> Update files:
>> - include/common.h
>> - lib/Makefile
>> - lib/uuid.c
>
> The patch already contains the list of changed files; there's little
> point duplicating this in the patch description.
>
>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>> cc: Stephen Warren <swarren@nvidia.com>
>> cc: trini@ti.com
>
> s/cc/Cc/
>
>> diff --git a/lib/Makefile b/lib/Makefile
>> index 70962b2..64a430f 100644
>> --- a/lib/Makefile
>> +++ b/lib/Makefile
>> @@ -59,10 +59,12 @@ obj-y += linux_string.o
>>   obj-$(CONFIG_REGEX) += slre.o
>>   obj-y += string.o
>>   obj-y += time.o
>> +obj-y += vsprintf.o
>
> Moving vsprintf.o seems entirely unrelated to this patch. If you want to
> clean that up, it should be a separate patch.
>
>>   obj-$(CONFIG_TRACE) += trace.o
>>   obj-$(CONFIG_BOOTP_PXE) += uuid.o
>>   obj-$(CONFIG_PARTITION_UUIDS) += uuid.o
>> -obj-y += vsprintf.o
>> +obj-$(CONFIG_RANDOM_UUID) += uuid.o
>> +obj-$(CONFIG_RANDOM_UUID) += rand.o
>>   obj-$(CONFIG_RANDOM_MACADDR) += rand.o
>
> I really hope listing the same object multiple times in obj-y is OK.
>
> Why not sort the lines you added so based on the config variable, so
>
>>   obj-$(CONFIG_RANDOM_MACADDR) += rand.o
>> +obj-$(CONFIG_RANDOM_UUID) += rand.o
>
> rather than:
>
>> +obj-$(CONFIG_RANDOM_UUID) += rand.o
>>   obj-$(CONFIG_RANDOM_MACADDR) += rand.o
>
>> diff --git a/lib/uuid.c b/lib/uuid.c
>> index 803bdcd..c0218ba 100644
>> --- a/lib/uuid.c
>> +++ b/lib/uuid.c
>> @@ -7,6 +7,29 @@
>>   #include <linux/ctype.h>
>>   #include <errno.h>
>>   #include <common.h>
>> +#include <part_efi.h>
>> +#include <malloc.h>
>> +
>> +#define UUID_STR_LEN			36
>> +#define UUID_STR_BYTE_LEN		37
>
> If UUID_STR_BYTE_LEN is the length in bytes, what units is UUID_STR_LEN
> in? I suppose the difference is the NULL terminator, but the names don't
> make that clear at all. I would suggest not defining UUID_STR_BYTE_LEN
> at all, but rather just writing "+ 1" at the appropriate place in the
> source code.
>
>> +#define UUID_BIN_BYTE_LEN		16
>
> Also, s/_BYTE// there.
>
>> +#define UUID_VERSION_CLEAR_BITS		0x0fff
>
> s/CLEAR_BITS/MASK/
>
>> +struct uuid {
>> +	unsigned int time_low;
>> +	unsigned short time_mid;
>> +	unsigned short time_hi_and_version;
>> +	unsigned char clock_seq_hi_and_reserved;
>> +	unsigned char clock_seq_low;
>> +	unsigned char node[6];
>> +};
>
> Is that structure definition endianness-safe?
>

UUID format is big-endian.
Actually for version 4 it doesn't matter because of it is random, and 
RFC says that version and variant are the most significant bits of 
proper structure field. In this code version and variant mask are stored 
at most significant bits - so this is big endian.
Actually we uses it as a string and as you can check in generated uuids 
its proper. As wiki says:
"Version 4 UUIDs have the form xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx 
where x is any hexadecimal digit and y is one of 8, 9, A, or B (e.g., 
f47ac10b-58cc-4372-a567-0e02b2c3d479)."

Even if this code runs on big-endian machine, version and variant are 
still set properly (most significant bits).

>> +/*
>> + * gen_rand_uuid() - this function generates 16 bytes len UUID V4 (randomly)
>> + *                   and stores it at a given pointer.
>
> I think "this function generates a random binary UUID v4, and stores it
> into the memory pointed at by the supplied pointer, which must be 16
> bytes in size" would be better.
>
>> +void gen_rand_uuid(unsigned char *uuid_bin)
>> +{
>> +	struct uuid *uuid = (struct uuid *)uuid_bin;
>> +	unsigned int *ptr = (unsigned int *)uuid_bin;
>> +	int i;
>> +
>> +	if (!uuid_bin)
>> +		return;
>
> I think you should either (a) assume NULL will never be passed to this
> function, or (b) return an error code if it happens. Silently failing to
> do anything doesn't make sense.
>
>> +	memset(uuid_bin, 0x0, sizeof(struct uuid));
>> +
>> +	/* Set all fields randomly */
>> +	for (i = 0; i < sizeof(struct uuid) / sizeof(*ptr); i++)
>> +		*(ptr + i) = rand();
>
> If the entire thing is filled randomly, why memset() the struct?
>
>
>> +/*
>> + * gen_rand_uuid_str() - this function generates UUID v4 (randomly)
>> + * into 36 character hexadecimal ASCII string representation of a 128-bit
>> + * (16 octets) UUID (Universally Unique Identifier) version 4 based on RFC4122
>> + * and stores it at a given pointer.
>
> There's a lot of duplication in that description. I think "this function
> generates a random string-formatted UUID v4, and stores it into the
> memory pointed at by the supplied pointer, which must be 37 bytes in
> size" would be better.
>
>> +void gen_rand_uuid_str(char *uuid_str)
>> +{
>> +	unsigned char uuid_bin[UUID_BIN_BYTE_LEN];
>> +
>> +	if (!uuid_str)
>> +		return;
>
> Again, I would drop that error-checking.
>

I also apply other comments to V3.
Thanks
Wolfgang Denk March 13, 2014, 6:41 p.m. UTC | #3
Dear Przemyslaw Marczak,

In message <cc0f558724a4d3ea3497b84601038f5f18f37a7b.1394037321.git.p.marczak@samsung.com> you wrote:
> This patch adds support to generate UUID (Universally Unique Identifier)
> in version 4 based on RFC4122, which is randomly.
...
> +struct uuid {
> +	unsigned int time_low;
> +	unsigned short time_mid;
> +	unsigned short time_hi_and_version;
> +	unsigned char clock_seq_hi_and_reserved;
> +	unsigned char clock_seq_low;
> +	unsigned char node[6];
> +};

This struct starts with an uint, so it requires alignment on a 32 bit
boundary (i. e. an address that is a multiple of 4).

> +void gen_rand_uuid(unsigned char *uuid_bin)
> +{
> +	struct uuid *uuid = (struct uuid *)uuid_bin;

Here you cast a pointer to the (unaligned) character buffer to a
struct buffer, which requires alignment.

> +	unsigned int *ptr = (unsigned int *)uuid_bin;

> +	/* Set all fields randomly */
> +	for (i = 0; i < sizeof(struct uuid) / sizeof(*ptr); i++)
> +		*(ptr + i) = rand();

This code is dangerous - if the size of the struct should not be a
multiple of sizeof(uint), there would remain uninitialized data.

And note that it is likely that all these accesses are unaligned and
might cause exceptions.

> +	/* Set V4 format */
> +	uuid->time_hi_and_version &= UUID_VERSION_CLEAR_BITS;
> +	uuid->time_hi_and_version |= UUID_VERSION << UUID_VERSION_SHIFT;

Potentially unaligned accesses.

Best regards,

Wolfgang Denk
Wolfgang Denk March 13, 2014, 6:41 p.m. UTC | #4
Dear Przemyslaw Marczak,

In message <5321F4AA.4000402@samsung.com> you wrote:
> 
> > Is that structure definition endianness-safe?
> 
> UUID format is big-endian.

OK.  Then you must make sure to store the data such that they result
in a big endian data format.

> Actually for version 4 it doesn't matter because of it is random, and 
> RFC says that version and variant are the most significant bits of 
> proper structure field. In this code version and variant mask are stored 
> at most significant bits - so this is big endian.
> Actually we uses it as a string and as you can check in generated uuids 
> its proper. As wiki says:
> "Version 4 UUIDs have the form xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx 
> where x is any hexadecimal digit and y is one of 8, 9, A, or B (e.g., 
> f47ac10b-58cc-4372-a567-0e02b2c3d479)."
> 
> Even if this code runs on big-endian machine, version and variant are 
> still set properly (most significant bits).

I don't see how come to this conclusion.  As far as I can see, you
initialize a binary data structure (where data storage _does_ depend
on the endianess), and then simply use this memory area - there is no
endianess handling anywhere.

Please also note that your code needs fixing due to alignment issues.
You cannot just cast a struct pointer which requires 32 bit alignment
to an arbitrary (i. e. unaligned) char pointer (comment to patch sent).

Best regards,

Wolfgang Denk
Tom Rini March 13, 2014, 7:18 p.m. UTC | #5
On Thu, Mar 13, 2014 at 07:41:24PM +0100, Wolfgang Denk wrote:
> Dear Przemyslaw Marczak,
> 
> In message <cc0f558724a4d3ea3497b84601038f5f18f37a7b.1394037321.git.p.marczak@samsung.com> you wrote:
> > This patch adds support to generate UUID (Universally Unique Identifier)
> > in version 4 based on RFC4122, which is randomly.
> ...
> > +struct uuid {
> > +	unsigned int time_low;
> > +	unsigned short time_mid;
> > +	unsigned short time_hi_and_version;
> > +	unsigned char clock_seq_hi_and_reserved;
> > +	unsigned char clock_seq_low;
> > +	unsigned char node[6];
> > +};
> 
> This struct starts with an uint, so it requires alignment on a 32 bit
> boundary (i. e. an address that is a multiple of 4).

And this needs to be marked as packed since we're using this as a direct
representation of things on-disk.

> > +void gen_rand_uuid(unsigned char *uuid_bin)
> > +{
> > +	struct uuid *uuid = (struct uuid *)uuid_bin;
> 
> Here you cast a pointer to the (unaligned) character buffer to a
> struct buffer, which requires alignment.

Potentially unaligned buffer.  There's only one caller thus far, and it
will be aligned there.  We do need to comment that the pointer needs to
be aligned.

> > +	unsigned int *ptr = (unsigned int *)uuid_bin;
> 
> > +	/* Set all fields randomly */
> > +	for (i = 0; i < sizeof(struct uuid) / sizeof(*ptr); i++)
> > +		*(ptr + i) = rand();
> 
> This code is dangerous - if the size of the struct should not be a
> multiple of sizeof(uint), there would remain uninitialized data.

With the struct not packed, it'll be padded out so this works.  But
looking at how we later use this as I say above, we do need to pack it,
and then this will not be safe.  Some looping of strncpy into the char
buffer, as a char so we whack the rand data in?

> > +	/* Set V4 format */
> > +	uuid->time_hi_and_version &= UUID_VERSION_CLEAR_BITS;
> > +	uuid->time_hi_and_version |= UUID_VERSION << UUID_VERSION_SHIFT;
> 
> Potentially unaligned accesses.

As-is no (hidden padding), with packed yes-but-handled (compiler can see
this too, will behave correctly).
Wolfgang Denk March 13, 2014, 7:48 p.m. UTC | #6
Dear Tom,

In message <20140313191814.GB16360@bill-the-cat> you wrote:
> 
> > > +struct uuid {
> > > +	unsigned int time_low;
> > > +	unsigned short time_mid;
> > > +	unsigned short time_hi_and_version;
> > > +	unsigned char clock_seq_hi_and_reserved;
> > > +	unsigned char clock_seq_low;
> > > +	unsigned char node[6];
> > > +};
> > 
> > This struct starts with an uint, so it requires alignment on a 32 bit
> > boundary (i. e. an address that is a multiple of 4).
>
> And this needs to be marked as packed since we're using this as a direct
> representation of things on-disk.

Not really.  The arrangement of the elemnts makes sure that there are
no inter-element gaps, and even if we had arrays of such structs
(which we don;t have) it would work as the struct adds up to a
multiple of 32 bits.

> > > +void gen_rand_uuid(unsigned char *uuid_bin)
> > > +{
> > > +	struct uuid *uuid = (struct uuid *)uuid_bin;
> > 
> > Here you cast a pointer to the (unaligned) character buffer to a
> > struct buffer, which requires alignment.
>
> Potentially unaligned buffer.  There's only one caller thus far, and it
> will be aligned there.  We do need to comment that the pointer needs to
> be aligned.

No.  We should rather fix the code such that it works with any
alignment.  It is trivial here to use a struct uuid on the stack and
then use memcpy() to cpy the data from the struct to the buffer - no
casting needed anywhere, and no alignment concerns either.

> With the struct not packed, it'll be padded out so this works.  But
> looking at how we later use this as I say above, we do need to pack it,
> and then this will not be safe.  Some looping of strncpy into the char
> buffer, as a char so we whack the rand data in?

I can't see why we would not simply initialize the elements step by
step.  It's just such a small struct.


Best regards,

Wolfgang Denk
Przemyslaw Marczak March 13, 2014, 7:51 p.m. UTC | #7
Hello,

On 03/13/2014 08:18 PM, Tom Rini wrote:
> On Thu, Mar 13, 2014 at 07:41:24PM +0100, Wolfgang Denk wrote:
>> Dear Przemyslaw Marczak,
>>
>> In message <cc0f558724a4d3ea3497b84601038f5f18f37a7b.1394037321.git.p.marczak@samsung.com> you wrote:
>>> This patch adds support to generate UUID (Universally Unique Identifier)
>>> in version 4 based on RFC4122, which is randomly.
>> ...
>>> +struct uuid {
>>> +	unsigned int time_low;
>>> +	unsigned short time_mid;
>>> +	unsigned short time_hi_and_version;
>>> +	unsigned char clock_seq_hi_and_reserved;
>>> +	unsigned char clock_seq_low;
>>> +	unsigned char node[6];
>>> +};
>>
>> This struct starts with an uint, so it requires alignment on a 32 bit
>> boundary (i. e. an address that is a multiple of 4).
>
> And this needs to be marked as packed since we're using this as a direct
> representation of things on-disk.
>

ok, I will add packed attribute.

>>> +void gen_rand_uuid(unsigned char *uuid_bin)
I can change this pointer above to unsigned int.

>>> +{
>>> +	struct uuid *uuid = (struct uuid *)uuid_bin;
>>
>> Here you cast a pointer to the (unaligned) character buffer to a
>> struct buffer, which requires alignment.
>
> Potentially unaligned buffer.  There's only one caller thus far, and it
> will be aligned there.  We do need to comment that the pointer needs to
> be aligned.
>
>>> +	unsigned int *ptr = (unsigned int *)uuid_bin;
>>
>>> +	/* Set all fields randomly */
>>> +	for (i = 0; i < sizeof(struct uuid) / sizeof(*ptr); i++)
>>> +		*(ptr + i) = rand();
>>
>> This code is dangerous - if the size of the struct should not be a
>> multiple of sizeof(uint), there would remain uninitialized data.
>

We know that uuid is 16bytes, so change the divider to "4" is better 
solution?

> With the struct not packed, it'll be padded out so this works.  But
> looking at how we later use this as I say above, we do need to pack it,
> and then this will not be safe.  Some looping of strncpy into the char
> buffer, as a char so we whack the rand data in?
>
>>> +	/* Set V4 format */
>>> +	uuid->time_hi_and_version &= UUID_VERSION_CLEAR_BITS;
>>> +	uuid->time_hi_and_version |= UUID_VERSION << UUID_VERSION_SHIFT;
>>
>> Potentially unaligned accesses.
Ok, I change this to clrsetbits.

>
> As-is no (hidden padding), with packed yes-but-handled (compiler can see
> this too, will behave correctly).
>
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

Thanks
Stephen Warren March 13, 2014, 7:55 p.m. UTC | #8
On 03/13/2014 01:48 PM, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20140313191814.GB16360@bill-the-cat> you wrote:
>>
>>>> +struct uuid {
>>>> +	unsigned int time_low;
>>>> +	unsigned short time_mid;
>>>> +	unsigned short time_hi_and_version;
>>>> +	unsigned char clock_seq_hi_and_reserved;
>>>> +	unsigned char clock_seq_low;
>>>> +	unsigned char node[6];
>>>> +};

>>>> +void gen_rand_uuid(unsigned char *uuid_bin)
>>>> +{
>>>> +	struct uuid *uuid = (struct uuid *)uuid_bin;
>>>
>>> Here you cast a pointer to the (unaligned) character buffer to a
>>> struct buffer, which requires alignment.
>>
>> Potentially unaligned buffer.  There's only one caller thus far, and it
>> will be aligned there.  We do need to comment that the pointer needs to
>> be aligned.
> 
> No.  We should rather fix the code such that it works with any
> alignment.  It is trivial here to use a struct uuid on the stack and
> then use memcpy() to cpy the data from the struct to the buffer - no
> casting needed anywhere, and no alignment concerns either.

Why not just change the prototype of the function so that it takes a
pointer to a struct uuid. That way, the compiler will ensure that
everything is aligned correctly (assuming there are no broken casts at
the call site), and we won't have to either document any assumptions
about alignment, nor perform a memcpy() to work around any misalignment.
diff mbox

Patch

diff --git a/include/common.h b/include/common.h
index 32377ad..20e9ae6 100644
--- a/include/common.h
+++ b/include/common.h
@@ -815,6 +815,8 @@  void	udelay        (unsigned long);
 void mdelay(unsigned long);
 
 /* lib/uuid.c */
+void gen_rand_uuid(unsigned char *uuid_bin);
+void gen_rand_uuid_str(char *uuid_str);
 void uuid_bin_to_str(unsigned char *uuid, char *str);
 int uuid_str_to_bin(char *uuid, unsigned char *out);
 int uuid_str_valid(const char *uuid);
@@ -831,7 +833,8 @@  char *	strmhz(char *buf, unsigned long hz);
 /* lib/rand.c */
 #if defined(CONFIG_RANDOM_MACADDR) || \
 	defined(CONFIG_BOOTP_RANDOM_DELAY) || \
-	defined(CONFIG_CMD_LINK_LOCAL)
+	defined(CONFIG_CMD_LINK_LOCAL) || \
+	defined(CONFIG_RANDOM_UUID)
 #define RAND_MAX -1U
 void srand(unsigned int seed);
 unsigned int rand(void);
diff --git a/lib/Makefile b/lib/Makefile
index 70962b2..64a430f 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -59,10 +59,12 @@  obj-y += linux_string.o
 obj-$(CONFIG_REGEX) += slre.o
 obj-y += string.o
 obj-y += time.o
+obj-y += vsprintf.o
 obj-$(CONFIG_TRACE) += trace.o
 obj-$(CONFIG_BOOTP_PXE) += uuid.o
 obj-$(CONFIG_PARTITION_UUIDS) += uuid.o
-obj-y += vsprintf.o
+obj-$(CONFIG_RANDOM_UUID) += uuid.o
+obj-$(CONFIG_RANDOM_UUID) += rand.o
 obj-$(CONFIG_RANDOM_MACADDR) += rand.o
 obj-$(CONFIG_BOOTP_RANDOM_DELAY) += rand.o
 obj-$(CONFIG_CMD_LINK_LOCAL) += rand.o
diff --git a/lib/uuid.c b/lib/uuid.c
index 803bdcd..c0218ba 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -7,6 +7,29 @@ 
 #include <linux/ctype.h>
 #include <errno.h>
 #include <common.h>
+#include <part_efi.h>
+#include <malloc.h>
+
+#define UUID_STR_LEN			36
+#define UUID_STR_BYTE_LEN		37
+#define UUID_BIN_BYTE_LEN		16
+
+#define UUID_VERSION_CLEAR_BITS		0x0fff
+#define UUID_VERSION_SHIFT		12
+#define UUID_VERSION			0x4
+
+#define UUID_VARIANT_CLEAR_BITS		0x3f
+#define UUID_VARIANT_SHIFT		7
+#define UUID_VARIANT			0x1
+
+struct uuid {
+	unsigned int time_low;
+	unsigned short time_mid;
+	unsigned short time_hi_and_version;
+	unsigned char clock_seq_hi_and_reserved;
+	unsigned char clock_seq_low;
+	unsigned char node[6];
+};
 
 /*
  * This is what a UUID string looks like.
@@ -94,3 +117,61 @@  void uuid_bin_to_str(unsigned char *uuid, char *str)
 		}
 	}
 }
+
+/*
+ * gen_rand_uuid() - this function generates 16 bytes len UUID V4 (randomly)
+ *                   and stores it at a given pointer.
+ *
+ * Layout of UUID Version 4:
+ * 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
+ *
+ * @param uuid_bin pointer to 16 bytes len array
+*/
+void gen_rand_uuid(unsigned char *uuid_bin)
+{
+	struct uuid *uuid = (struct uuid *)uuid_bin;
+	unsigned int *ptr = (unsigned int *)uuid_bin;
+	int i;
+
+	if (!uuid_bin)
+		return;
+
+	memset(uuid_bin, 0x0, sizeof(struct uuid));
+
+	/* Set all fields randomly */
+	for (i = 0; i < sizeof(struct uuid) / sizeof(*ptr); i++)
+		*(ptr + i) = rand();
+
+	/* Set V4 format */
+	uuid->time_hi_and_version &= UUID_VERSION_CLEAR_BITS;
+	uuid->time_hi_and_version |= UUID_VERSION << UUID_VERSION_SHIFT;
+
+	uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_CLEAR_BITS;
+	uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
+}
+
+/*
+ * gen_rand_uuid_str() - this function generates UUID v4 (randomly)
+ * into 36 character hexadecimal ASCII string representation of a 128-bit
+ * (16 octets) UUID (Universally Unique Identifier) version 4 based on RFC4122
+ * and stores it at a given pointer.
+ *
+ * @param uuid_str pointer to 37 bytes len array
+ */
+void gen_rand_uuid_str(char *uuid_str)
+{
+	unsigned char uuid_bin[UUID_BIN_BYTE_LEN];
+
+	if (!uuid_str)
+		return;
+
+	gen_rand_uuid(uuid_bin);
+
+	uuid_bin_to_str(uuid_bin, uuid_str);
+}