[PATCH v1 0/2] uuid: more users to use generic UUID API

[PATCH v1 0/2] uuid: more users to use generic UUID API

[Andy Shevchenko]

Andrew, there are two more users converted as well which makes whole excersice
quite meaningful.

Andy Shevchenko (2):
  LDM: use generic UUID library
  wmi: use generic UUID library

 block/partitions/ldm.c     |  60 ++------------------------
 drivers/platform/x86/wmi.c | 104 ++++++---------------------------------------
 2 files changed, 17 insertions(+), 147 deletions(-)

-- 
2.8.0.rc3

[PATCH v1 1/2] LDM: use generic UUID library

[Andy Shevchenko]

Instead of opencoding let's use generic UUID library functions here.

Cc: "Richard Russon (FlatCap)" <ldm@flatcap.org>
Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 block/partitions/ldm.c | 60 ++++----------------------------------------------
 1 file changed, 4 insertions(+), 56 deletions(-)

diff --git a/block/partitions/ldm.c b/block/partitions/ldm.c
index e507cfb..edcea70 100644
--- a/block/partitions/ldm.c
+++ b/block/partitions/ldm.c
@@ -27,6 +27,8 @@
 #include <linux/pagemap.h>
 #include <linux/stringify.h>
 #include <linux/kernel.h>
+#include <linux/uuid.h>
+
 #include "ldm.h"
 #include "check.h"
 #include "msdos.h"
@@ -66,60 +68,6 @@ void _ldm_printk(const char *level, const char *function, const char *fmt, ...)
 }
 
 /**
- * ldm_parse_hexbyte - Convert a ASCII hex number to a byte
- * @src:  Pointer to at least 2 characters to convert.
- *
- * Convert a two character ASCII hex string to a number.
- *
- * Return:  0-255  Success, the byte was parsed correctly
- *          -1     Error, an invalid character was supplied
- */
-static int ldm_parse_hexbyte (const u8 *src)
-{
-	unsigned int x;		/* For correct wrapping */
-	int h;
-
-	/* high part */
-	x = h = hex_to_bin(src[0]);
-	if (h < 0)
-		return -1;
-
-	/* low part */
-	h = hex_to_bin(src[1]);
-	if (h < 0)
-		return -1;
-
-	return (x << 4) + h;
-}
-
-/**
- * ldm_parse_guid - Convert GUID from ASCII to binary
- * @src:   36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
- * @dest:  Memory block to hold binary GUID (16 bytes)
- *
- * N.B. The GUID need not be NULL terminated.
- *
- * Return:  'true'   @dest contains binary GUID
- *          'false'  @dest contents are undefined
- */
-static bool ldm_parse_guid (const u8 *src, u8 *dest)
-{
-	static const int size[] = { 4, 2, 2, 2, 6 };
-	int i, j, v;
-
-	if (src[8]  != '-' || src[13] != '-' ||
-	    src[18] != '-' || src[23] != '-')
-		return false;
-
-	for (j = 0; j < 5; j++, src++)
-		for (i = 0; i < size[j]; i++, src+=2, *dest++ = v)
-			if ((v = ldm_parse_hexbyte (src)) < 0)
-				return false;
-
-	return true;
-}
-
-/**
  * ldm_parse_privhead - Read the LDM Database PRIVHEAD structure
  * @data:  Raw database PRIVHEAD structure loaded from the device
  * @ph:    In-memory privhead structure in which to return parsed information
@@ -167,7 +115,7 @@ static bool ldm_parse_privhead(const u8 *data, struct privhead *ph)
 		ldm_error("PRIVHEAD disk size doesn't match real disk size");
 		return false;
 	}
-	if (!ldm_parse_guid(data + 0x0030, ph->disk_id)) {
+	if (uuid_be_to_bin(data + 0x0030, (uuid_be *)ph->disk_id)) {
 		ldm_error("PRIVHEAD contains an invalid GUID.");
 		return false;
 	}
@@ -944,7 +892,7 @@ static bool ldm_parse_dsk3 (const u8 *buffer, int buflen, struct vblk *vb)
 	disk = &vb->vblk.disk;
 	ldm_get_vstr (buffer + 0x18 + r_diskid, disk->alt_name,
 		sizeof (disk->alt_name));
-	if (!ldm_parse_guid (buffer + 0x19 + r_name, disk->disk_id))
+	if (uuid_be_to_bin(buffer + 0x19 + r_name, (uuid_be *)disk->disk_id))
 		return false;
 
 	return true;
-- 
2.8.0.rc3

[PATCH v1 2/2] wmi: use generic UUID library

[Andy Shevchenko]

Instead of opencoding let's use generic UUID library functions here.

Cc: Darren Hart <dvhart@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/platform/x86/wmi.c | 104 ++++++---------------------------------------
 1 file changed, 13 insertions(+), 91 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index eb391a2..ceeb8c1 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -37,6 +37,7 @@
 #include <linux/acpi.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/uuid.h>
 
 ACPI_MODULE_NAME("wmi");
 MODULE_AUTHOR("Carlos Corbacho");
@@ -115,100 +116,21 @@ static struct acpi_driver acpi_wmi_driver = {
  * GUID parsing functions
  */
 
-/**
- * wmi_parse_hexbyte - Convert a ASCII hex number to a byte
- * @src:  Pointer to at least 2 characters to convert.
- *
- * Convert a two character ASCII hex string to a number.
- *
- * Return:  0-255  Success, the byte was parsed correctly
- *          -1     Error, an invalid character was supplied
- */
-static int wmi_parse_hexbyte(const u8 *src)
-{
-	int h;
-	int value;
-
-	/* high part */
-	h = value = hex_to_bin(src[0]);
-	if (value < 0)
-		return -1;
-
-	/* low part */
-	value = hex_to_bin(src[1]);
-	if (value >= 0)
-		return (h << 4) | value;
-	return -1;
-}
-
-/**
- * wmi_swap_bytes - Rearrange GUID bytes to match GUID binary
- * @src:   Memory block holding binary GUID (16 bytes)
- * @dest:  Memory block to hold byte swapped binary GUID (16 bytes)
- *
- * Byte swap a binary GUID to match it's real GUID value
- */
-static void wmi_swap_bytes(u8 *src, u8 *dest)
-{
-	int i;
-
-	for (i = 0; i <= 3; i++)
-		memcpy(dest + i, src + (3 - i), 1);
-
-	for (i = 0; i <= 1; i++)
-		memcpy(dest + 4 + i, src + (5 - i), 1);
-
-	for (i = 0; i <= 1; i++)
-		memcpy(dest + 6 + i, src + (7 - i), 1);
-
-	memcpy(dest + 8, src + 8, 8);
-}
-
-/**
- * wmi_parse_guid - Convert GUID from ASCII to binary
- * @src:   36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
- * @dest:  Memory block to hold binary GUID (16 bytes)
- *
- * N.B. The GUID need not be NULL terminated.
- *
- * Return:  'true'   @dest contains binary GUID
- *          'false'  @dest contents are undefined
- */
-static bool wmi_parse_guid(const u8 *src, u8 *dest)
-{
-	static const int size[] = { 4, 2, 2, 2, 6 };
-	int i, j, v;
-
-	if (src[8]  != '-' || src[13] != '-' ||
-		src[18] != '-' || src[23] != '-')
-		return false;
-
-	for (j = 0; j < 5; j++, src++) {
-		for (i = 0; i < size[j]; i++, src += 2, *dest++ = v) {
-			v = wmi_parse_hexbyte(src);
-			if (v < 0)
-				return false;
-		}
-	}
-
-	return true;
-}
-
 static bool find_guid(const char *guid_string, struct wmi_block **out)
 {
-	char tmp[16], guid_input[16];
+	uuid_le guid_input;
 	struct wmi_block *wblock;
 	struct guid_block *block;
 	struct list_head *p;
 
-	wmi_parse_guid(guid_string, tmp);
-	wmi_swap_bytes(tmp, guid_input);
+	if (uuid_le_to_bin(guid_string, &guid_input))
+		return false;
 
 	list_for_each(p, &wmi_block_list) {
 		wblock = list_entry(p, struct wmi_block, list);
 		block = &wblock->gblock;
 
-		if (memcmp(block->guid, guid_input, 16) == 0) {
+		if (memcmp(block->guid, &guid_input, 16) == 0) {
 			if (out)
 				*out = wblock;
 			return true;
@@ -498,20 +420,20 @@ wmi_notify_handler handler, void *data)
 {
 	struct wmi_block *block;
 	acpi_status status = AE_NOT_EXIST;
-	char tmp[16], guid_input[16];
+	uuid_le guid_input;
 	struct list_head *p;
 
 	if (!guid || !handler)
 		return AE_BAD_PARAMETER;
 
-	wmi_parse_guid(guid, tmp);
-	wmi_swap_bytes(tmp, guid_input);
+	if (uuid_le_to_bin(guid, &guid_input))
+		return AE_BAD_PARAMETER;
 
 	list_for_each(p, &wmi_block_list) {
 		acpi_status wmi_status;
 		block = list_entry(p, struct wmi_block, list);
 
-		if (memcmp(block->gblock.guid, guid_input, 16) == 0) {
+		if (memcmp(block->gblock.guid, &guid_input, 16) == 0) {
 			if (block->handler &&
 			    block->handler != wmi_notify_debug)
 				return AE_ALREADY_ACQUIRED;
@@ -539,20 +461,20 @@ acpi_status wmi_remove_notify_handler(const char *guid)
 {
 	struct wmi_block *block;
 	acpi_status status = AE_NOT_EXIST;
-	char tmp[16], guid_input[16];
+	uuid_le guid_input;
 	struct list_head *p;
 
 	if (!guid)
 		return AE_BAD_PARAMETER;
 
-	wmi_parse_guid(guid, tmp);
-	wmi_swap_bytes(tmp, guid_input);
+	if (uuid_le_to_bin(guid, &guid_input))
+		return AE_BAD_PARAMETER;
 
 	list_for_each(p, &wmi_block_list) {
 		acpi_status wmi_status;
 		block = list_entry(p, struct wmi_block, list);
 
-		if (memcmp(block->gblock.guid, guid_input, 16) == 0) {
+		if (memcmp(block->gblock.guid, &guid_input, 16) == 0) {
 			if (!block->handler ||
 			    block->handler == wmi_notify_debug)
 				return AE_NULL_ENTRY;
-- 
2.8.0.rc3

Re: [PATCH v1 2/2] wmi: use generic UUID library

[Darren Hart]

On Thu, Apr 07, 2016 at 08:00:55PM +0300, Andy Shevchenko wrote:
> Instead of opencoding let's use generic UUID library functions here.
> 
> Cc: Darren Hart <dvhart@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/platform/x86/wmi.c | 104 ++++++---------------------------------------
>  1 file changed, 13 insertions(+), 91 deletions(-)
> 

My favorite kind of patches :-)

The only functional difference I detected was an additional check/errcode-return
in uuid_le_to_bin, but this is a bug fix in and of itself.

I'm happy to see this go in once the details of the generic uuid library as
discussed in the related threads are ironed out.

Andrew, will you be pulling this in as part of that series?

Reviewed-by: Darren Hart <dvhart@linux.intel.com>

> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index eb391a2..ceeb8c1 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -37,6 +37,7 @@
>  #include <linux/acpi.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> +#include <linux/uuid.h>
>  
>  ACPI_MODULE_NAME("wmi");
>  MODULE_AUTHOR("Carlos Corbacho");
> @@ -115,100 +116,21 @@ static struct acpi_driver acpi_wmi_driver = {
>   * GUID parsing functions
>   */
>  
> -/**
> - * wmi_parse_hexbyte - Convert a ASCII hex number to a byte
> - * @src:  Pointer to at least 2 characters to convert.
> - *
> - * Convert a two character ASCII hex string to a number.
> - *
> - * Return:  0-255  Success, the byte was parsed correctly
> - *          -1     Error, an invalid character was supplied
> - */
> -static int wmi_parse_hexbyte(const u8 *src)
> -{
> -	int h;
> -	int value;
> -
> -	/* high part */
> -	h = value = hex_to_bin(src[0]);
> -	if (value < 0)
> -		return -1;
> -
> -	/* low part */
> -	value = hex_to_bin(src[1]);
> -	if (value >= 0)
> -		return (h << 4) | value;
> -	return -1;
> -}
> -
> -/**
> - * wmi_swap_bytes - Rearrange GUID bytes to match GUID binary
> - * @src:   Memory block holding binary GUID (16 bytes)
> - * @dest:  Memory block to hold byte swapped binary GUID (16 bytes)
> - *
> - * Byte swap a binary GUID to match it's real GUID value
> - */
> -static void wmi_swap_bytes(u8 *src, u8 *dest)
> -{
> -	int i;
> -
> -	for (i = 0; i <= 3; i++)
> -		memcpy(dest + i, src + (3 - i), 1);
> -
> -	for (i = 0; i <= 1; i++)
> -		memcpy(dest + 4 + i, src + (5 - i), 1);
> -
> -	for (i = 0; i <= 1; i++)
> -		memcpy(dest + 6 + i, src + (7 - i), 1);
> -
> -	memcpy(dest + 8, src + 8, 8);
> -}
> -
> -/**
> - * wmi_parse_guid - Convert GUID from ASCII to binary
> - * @src:   36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
> - * @dest:  Memory block to hold binary GUID (16 bytes)
> - *
> - * N.B. The GUID need not be NULL terminated.
> - *
> - * Return:  'true'   @dest contains binary GUID
> - *          'false'  @dest contents are undefined
> - */
> -static bool wmi_parse_guid(const u8 *src, u8 *dest)
> -{
> -	static const int size[] = { 4, 2, 2, 2, 6 };
> -	int i, j, v;
> -
> -	if (src[8]  != '-' || src[13] != '-' ||
> -		src[18] != '-' || src[23] != '-')
> -		return false;
> -
> -	for (j = 0; j < 5; j++, src++) {
> -		for (i = 0; i < size[j]; i++, src += 2, *dest++ = v) {
> -			v = wmi_parse_hexbyte(src);
> -			if (v < 0)
> -				return false;
> -		}
> -	}
> -
> -	return true;
> -}
> -
>  static bool find_guid(const char *guid_string, struct wmi_block **out)
>  {
> -	char tmp[16], guid_input[16];
> +	uuid_le guid_input;
>  	struct wmi_block *wblock;
>  	struct guid_block *block;
>  	struct list_head *p;
>  
> -	wmi_parse_guid(guid_string, tmp);
> -	wmi_swap_bytes(tmp, guid_input);
> +	if (uuid_le_to_bin(guid_string, &guid_input))
> +		return false;
>  
>  	list_for_each(p, &wmi_block_list) {
>  		wblock = list_entry(p, struct wmi_block, list);
>  		block = &wblock->gblock;
>  
> -		if (memcmp(block->guid, guid_input, 16) == 0) {
> +		if (memcmp(block->guid, &guid_input, 16) == 0) {
>  			if (out)
>  				*out = wblock;
>  			return true;
> @@ -498,20 +420,20 @@ wmi_notify_handler handler, void *data)
>  {
>  	struct wmi_block *block;
>  	acpi_status status = AE_NOT_EXIST;
> -	char tmp[16], guid_input[16];
> +	uuid_le guid_input;
>  	struct list_head *p;
>  
>  	if (!guid || !handler)
>  		return AE_BAD_PARAMETER;
>  
> -	wmi_parse_guid(guid, tmp);
> -	wmi_swap_bytes(tmp, guid_input);
> +	if (uuid_le_to_bin(guid, &guid_input))
> +		return AE_BAD_PARAMETER;
>  
>  	list_for_each(p, &wmi_block_list) {
>  		acpi_status wmi_status;
>  		block = list_entry(p, struct wmi_block, list);
>  
> -		if (memcmp(block->gblock.guid, guid_input, 16) == 0) {
> +		if (memcmp(block->gblock.guid, &guid_input, 16) == 0) {
>  			if (block->handler &&
>  			    block->handler != wmi_notify_debug)
>  				return AE_ALREADY_ACQUIRED;
> @@ -539,20 +461,20 @@ acpi_status wmi_remove_notify_handler(const char *guid)
>  {
>  	struct wmi_block *block;
>  	acpi_status status = AE_NOT_EXIST;
> -	char tmp[16], guid_input[16];
> +	uuid_le guid_input;
>  	struct list_head *p;
>  
>  	if (!guid)
>  		return AE_BAD_PARAMETER;
>  
> -	wmi_parse_guid(guid, tmp);
> -	wmi_swap_bytes(tmp, guid_input);
> +	if (uuid_le_to_bin(guid, &guid_input))
> +		return AE_BAD_PARAMETER;
>  
>  	list_for_each(p, &wmi_block_list) {
>  		acpi_status wmi_status;
>  		block = list_entry(p, struct wmi_block, list);
>  
> -		if (memcmp(block->gblock.guid, guid_input, 16) == 0) {
> +		if (memcmp(block->gblock.guid, &guid_input, 16) == 0) {
>  			if (!block->handler ||
>  			    block->handler == wmi_notify_debug)
>  				return AE_NULL_ENTRY;
> -- 
> 2.8.0.rc3
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center