Hello Stephen,
Thanks for review again:)

On 03/25/2014 08:12 PM, Stephen Warren wrote:
On 03/19/2014 11:58 AM, Przemyslaw Marczak wrote:
Changes in lib/uuid.c to:
- uuid_str_to_bin()
- uuid_bin_to_str()

New parameter is added to specify input/output string format in listed functions
This change allows easy recognize which UUID type is or should be stored in 
given
string array. Binary data of UUID and GUID is always stored in big endian, only
string representations are different as follows.

String byte: 0                                  36
String char: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
string UUID:    be     be   be   be       be
string GUID:    le     le   le   be       be

This patch also updates functions calls and declarations in a whole code.

Ah, this patch pretty much solves all the comments I had on patch 1/6,
so feel free to ignore those.

Just a couple minor points below, but otherwise, patches 1 and 2,
Acked-by: Stephen Warren <swar...@nvidia.com>

Ok, thank you.

diff --git a/include/uuid.h b/include/uuid.h

+typedef enum {
+       UUID_STR_FORMAT_STD,
+       UUID_STR_FORMAT_GUID
+} uuid_str_t;

I would rename "STD" to "UUID"; after all, someone wanting to use GUIDs
might think /that/ is the standard format:-)

But this is a bit bike-sheddy/nit-picky, so if you don't want to I won't
object.


Actually I think that UUID_STR_FORMAT_UUID gives no information that this the main format of UUID, so I prefer to leave STD.

diff --git a/lib/uuid.c b/lib/uuid.c


+void uuid_bin_to_str(unsigned char *uuid_bin, char *uuid_str,
+                    uuid_str_t str_format)
  {
-       static const u8 le[16] = {3, 2, 1, 0, 5, 4, 7, 6, 8, 9, 10, 11,
-                                 12, 13, 14, 15};
+       const u8 uuid_char_order[UUID_BIN_LEN] = {0, 1, 2, 3, 4, 5, 6, 7, 8,
+                                                 9, 10, 11, 12, 13, 14, 15};
+       const u8 guid_char_order[UUID_BIN_LEN] = {3, 2, 1, 0, 5, 4, 7, 6, 8,
+                                                 9, 10, 11, 12, 13, 14, 15};

These are really more binary data order than char order, since each one
of the bytes pointed at by entries in these arrays ends up being 2
characters. s/char/bin/ in the variable names perhaps?


Yes, you are right. But according to the specification UUID and UUID bin format are always in big-endian - only bytes in some STRING blocks have different order. This works in two ways but to be consistent with specification I called this as "uuid_char_order". And this is directly used by sprintf: "sprintf(uuid_str, "%02x", uuid_bin[char_order[i]]);".

+       const u8 *char_order;
        int i;

+       /*
+        * UUID and GUID bin data - always in big endian:
+        * 4B-2B-2B-2B-6B
+        * be be be be be

Strings don't really have an endianness, since they're already byte
data. Rather than endianness, you really mean "normal numerical digit
ordering". This comment also applies to the description of UUID string
formats in patch 1/6.

Right but the comment above says about "bin" data (16B len).

Thanks
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to