From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Wed, 26 Mar 2014 13:00:40 +0100 Subject: [U-Boot] [PATCH v4 2/6] lib: uuid: code refactor for proper maintain between uuid bin and string In-Reply-To: <5331D538.60102@wwwdotorg.org> References: <5ef7cdb8df4fb05c3c371e29d7a61e28e1563a68.1394807506.git.p.marczak@samsung.com> <1395251911-26540-1-git-send-email-p.marczak@samsung.com> <1395251911-26540-2-git-send-email-p.marczak@samsung.com> <5331D538.60102@wwwdotorg.org> Message-ID: <5332C168.4030308@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > 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.marczak at samsung.com