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 swarren@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 beStrings 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