All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] uuid: convert users to generic UUID API
@ 2016-04-04 13:30 ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel, linux-efi, linux-api
  Cc: Andy Shevchenko

There are few functions here and there along with type definitions that provide
UUID API. This series consolidates everything under one hood and converts
current users.

This has been tested for a while internally, however it doesn't mean we covered
all possible cases (especially accuracy of UUID constants after conversion).
So, please test this as much as you can and provide your tag. We appreciate the
effort.

Since v1:
- address Matt's comment (fix return value of efivarfs_valid_name())
- drop patches 5 and 6 (ACPI) for now, will return to them after sorting out generic things
- rebase on top of latest linux-next

Andy Shevchenko (8):
  lib/vsprintf: simplify UUID printing
  lib/uuid: move generate_random_uuid() to uuid.c
  lib/uuid: introduce few more generic helpers for UUID
  lib/uuid: remove FSF address
  sysctl: drop away useless label
  sysctl: use generic UUID library
  efi: redefine type, constant, macro from generic code
  efivars: use generic UUID library

 drivers/char/random.c     | 21 +----------
 fs/btrfs/volumes.c        |  2 +-
 fs/efivarfs/inode.c       | 40 ++------------------
 fs/ext4/ioctl.c           |  2 +-
 fs/f2fs/file.c            |  2 +-
 fs/reiserfs/objectid.c    |  2 +-
 fs/ubifs/sb.c             |  2 +-
 include/linux/efi.h       | 14 ++-----
 include/linux/random.h    |  1 -
 include/linux/uuid.h      | 21 ++++++++---
 include/uapi/linux/uuid.h |  4 --
 kernel/sysctl_binary.c    | 30 +++++----------
 lib/uuid.c                | 96 ++++++++++++++++++++++++++++++++++++++++++++---
 lib/vsprintf.c            | 21 ++++-------
 14 files changed, 137 insertions(+), 121 deletions(-)

-- 
2.8.0.rc3

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 0/8] uuid: convert users to generic UUID API
@ 2016-04-04 13:30 ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
  Cc: Andy Shevchenko

There are few functions here and there along with type definitions that provide
UUID API. This series consolidates everything under one hood and converts
current users.

This has been tested for a while internally, however it doesn't mean we covered
all possible cases (especially accuracy of UUID constants after conversion).
So, please test this as much as you can and provide your tag. We appreciate the
effort.

Since v1:
- address Matt's comment (fix return value of efivarfs_valid_name())
- drop patches 5 and 6 (ACPI) for now, will return to them after sorting out generic things
- rebase on top of latest linux-next

Andy Shevchenko (8):
  lib/vsprintf: simplify UUID printing
  lib/uuid: move generate_random_uuid() to uuid.c
  lib/uuid: introduce few more generic helpers for UUID
  lib/uuid: remove FSF address
  sysctl: drop away useless label
  sysctl: use generic UUID library
  efi: redefine type, constant, macro from generic code
  efivars: use generic UUID library

 drivers/char/random.c     | 21 +----------
 fs/btrfs/volumes.c        |  2 +-
 fs/efivarfs/inode.c       | 40 ++------------------
 fs/ext4/ioctl.c           |  2 +-
 fs/f2fs/file.c            |  2 +-
 fs/reiserfs/objectid.c    |  2 +-
 fs/ubifs/sb.c             |  2 +-
 include/linux/efi.h       | 14 ++-----
 include/linux/random.h    |  1 -
 include/linux/uuid.h      | 21 ++++++++---
 include/uapi/linux/uuid.h |  4 --
 kernel/sysctl_binary.c    | 30 +++++----------
 lib/uuid.c                | 96 ++++++++++++++++++++++++++++++++++++++++++++---
 lib/vsprintf.c            | 21 ++++-------
 14 files changed, 137 insertions(+), 121 deletions(-)

-- 
2.8.0.rc3

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 1/8] lib/vsprintf: simplify UUID printing
  2016-04-04 13:30 ` Andy Shevchenko
  (?)
@ 2016-04-04 13:30 ` Andy Shevchenko
  -1 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel, linux-efi, linux-api
  Cc: Andy Shevchenko

Since we have hex_byte_pack_upper() we may use it directly and avoid second
loop.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/vsprintf.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index ccb664b..be0e7cf 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1324,7 +1324,10 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
 	}
 
 	for (i = 0; i < 16; i++) {
-		p = hex_byte_pack(p, addr[index[i]]);
+		if (uc)
+			p = hex_byte_pack_upper(p, addr[index[i]]);
+		else
+			p = hex_byte_pack(p, addr[index[i]]);
 		switch (i) {
 		case 3:
 		case 5:
@@ -1337,13 +1340,6 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
 
 	*p = 0;
 
-	if (uc) {
-		p = uuid;
-		do {
-			*p = toupper(*p);
-		} while (*(++p));
-	}
-
 	return string(buf, end, uuid, spec);
 }
 
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 2/8] lib/uuid: move generate_random_uuid() to uuid.c
  2016-04-04 13:30 ` Andy Shevchenko
  (?)
  (?)
@ 2016-04-04 13:30 ` Andy Shevchenko
  -1 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel, linux-efi, linux-api
  Cc: Andy Shevchenko

Let's gather the UUID related functions under one hood.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/char/random.c  | 21 +--------------------
 fs/btrfs/volumes.c     |  2 +-
 fs/ext4/ioctl.c        |  2 +-
 fs/f2fs/file.c         |  2 +-
 fs/reiserfs/objectid.c |  2 +-
 fs/ubifs/sb.c          |  2 +-
 include/linux/random.h |  1 -
 include/linux/uuid.h   |  2 ++
 lib/uuid.c             | 20 ++++++++++++++++++++
 9 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index b583e53..0158d3b 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -260,6 +260,7 @@
 #include <linux/irq.h>
 #include <linux/syscalls.h>
 #include <linux/completion.h>
+#include <linux/uuid.h>
 
 #include <asm/processor.h>
 #include <asm/uaccess.h>
@@ -1621,26 +1622,6 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 	return urandom_read(NULL, buf, count, NULL);
 }
 
-/***************************************************************
- * Random UUID interface
- *
- * Used here for a Boot ID, but can be useful for other kernel
- * drivers.
- ***************************************************************/
-
-/*
- * Generate random UUID
- */
-void generate_random_uuid(unsigned char uuid_out[16])
-{
-	get_random_bytes(uuid_out, 16);
-	/* Set UUID version to 4 --- truly random generation */
-	uuid_out[6] = (uuid_out[6] & 0x0F) | 0x40;
-	/* Set the UUID variant to DCE */
-	uuid_out[8] = (uuid_out[8] & 0x3F) | 0x80;
-}
-EXPORT_SYMBOL(generate_random_uuid);
-
 /********************************************************************
  *
  * Sysctl interface
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 09ca950..7ffeb9b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -20,13 +20,13 @@
 #include <linux/slab.h>
 #include <linux/buffer_head.h>
 #include <linux/blkdev.h>
-#include <linux/random.h>
 #include <linux/iocontext.h>
 #include <linux/capability.h>
 #include <linux/ratelimit.h>
 #include <linux/kthread.h>
 #include <linux/raid/pq.h>
 #include <linux/semaphore.h>
+#include <linux/uuid.h>
 #include <asm/div64.h>
 #include "ctree.h"
 #include "extent_map.h"
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index eae5917..7497f50 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -13,8 +13,8 @@
 #include <linux/compat.h>
 #include <linux/mount.h>
 #include <linux/file.h>
-#include <linux/random.h>
 #include <linux/quotaops.h>
+#include <linux/uuid.h>
 #include <asm/uaccess.h>
 #include "ext4_jbd2.h"
 #include "ext4.h"
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index b41c357..ccd0742 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -20,7 +20,7 @@
 #include <linux/uaccess.h>
 #include <linux/mount.h>
 #include <linux/pagevec.h>
-#include <linux/random.h>
+#include <linux/uuid.h>
 
 #include "f2fs.h"
 #include "node.h"
diff --git a/fs/reiserfs/objectid.c b/fs/reiserfs/objectid.c
index 99a5d5d..415d66c 100644
--- a/fs/reiserfs/objectid.c
+++ b/fs/reiserfs/objectid.c
@@ -3,8 +3,8 @@
  */
 
 #include <linux/string.h>
-#include <linux/random.h>
 #include <linux/time.h>
+#include <linux/uuid.h>
 #include "reiserfs.h"
 
 /* find where objectid map starts */
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index f4fbc7b..3cbb904 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -28,8 +28,8 @@
 
 #include "ubifs.h"
 #include <linux/slab.h>
-#include <linux/random.h>
 #include <linux/math64.h>
+#include <linux/uuid.h>
 
 /*
  * Default journal size in logical eraseblocks as a percent of total
diff --git a/include/linux/random.h b/include/linux/random.h
index 9c29122..e47e533 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -26,7 +26,6 @@ extern void get_random_bytes(void *buf, int nbytes);
 extern int add_random_ready_callback(struct random_ready_callback *rdy);
 extern void del_random_ready_callback(struct random_ready_callback *rdy);
 extern void get_random_bytes_arch(void *buf, int nbytes);
-void generate_random_uuid(unsigned char uuid_out[16]);
 extern int random_int_secret_init(void);
 
 #ifndef MODULE
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 6df2509..91c2b6d 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -33,6 +33,8 @@ static inline int uuid_be_cmp(const uuid_be u1, const uuid_be u2)
 	return memcmp(&u1, &u2, sizeof(uuid_be));
 }
 
+void generate_random_uuid(unsigned char uuid[16]);
+
 extern void uuid_le_gen(uuid_le *u);
 extern void uuid_be_gen(uuid_be *u);
 
diff --git a/lib/uuid.c b/lib/uuid.c
index 398821e..6c81c0b 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -23,6 +23,26 @@
 #include <linux/uuid.h>
 #include <linux/random.h>
 
+/***************************************************************
+ * Random UUID interface
+ *
+ * Used here for a Boot ID, but can be useful for other kernel
+ * drivers.
+ ***************************************************************/
+
+/*
+ * Generate random UUID
+ */
+void generate_random_uuid(unsigned char uuid[16])
+{
+	get_random_bytes(uuid, 16);
+	/* Set UUID version to 4 --- truly random generation */
+	uuid[6] = (uuid[6] & 0x0F) | 0x40;
+	/* Set the UUID variant to DCE */
+	uuid[8] = (uuid[8] & 0x3F) | 0x80;
+}
+EXPORT_SYMBOL(generate_random_uuid);
+
 static void __uuid_gen_common(__u8 b[16])
 {
 	prandom_bytes(b, 16);
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 3/8] lib/uuid: introduce few more generic helpers for UUID
  2016-04-04 13:30 ` Andy Shevchenko
                   ` (2 preceding siblings ...)
  (?)
@ 2016-04-04 13:30 ` Andy Shevchenko
  2016-04-04 23:40   ` Andrew Morton
  -1 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel, linux-efi, linux-api
  Cc: Andy Shevchenko

There are new helpers in this patch:

uuid_is_valid		checks if a UUID is valid
uuid_be_to_bin		converts from string to binary (big endian)
uuid_le_to_bin		converts from string to binary (little endian)

They will be used in future, i.e. in the following patches in the series.

This also moves indices arrays to lib/uuid.c to be shared accross modules.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/uuid.h | 13 ++++++++++
 lib/uuid.c           | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/vsprintf.c       |  9 +++----
 3 files changed, 87 insertions(+), 5 deletions(-)

diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 91c2b6d..616347f 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -22,6 +22,11 @@
 
 #include <uapi/linux/uuid.h>
 
+/*
+ * The length of a UUID string ("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
+ * not including trailing NUL.
+ */
+#define	UUID_STRING_LEN		36
 
 static inline int uuid_le_cmp(const uuid_le u1, const uuid_le u2)
 {
@@ -38,4 +43,12 @@ void generate_random_uuid(unsigned char uuid[16]);
 extern void uuid_le_gen(uuid_le *u);
 extern void uuid_be_gen(uuid_be *u);
 
+int __must_check uuid_is_valid(const char *uuid);
+
+extern const u8 uuid_le_index[16];
+extern const u8 uuid_be_index[16];
+
+int uuid_le_to_bin(const char *uuid, uuid_le *u);
+int uuid_be_to_bin(const char *uuid, uuid_be *u);
+
 #endif
diff --git a/lib/uuid.c b/lib/uuid.c
index 6c81c0b..995865c 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -19,10 +19,17 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/ctype.h>
+#include <linux/errno.h>
 #include <linux/export.h>
 #include <linux/uuid.h>
 #include <linux/random.h>
 
+const u8 uuid_le_index[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
+EXPORT_SYMBOL(uuid_le_index);
+const u8 uuid_be_index[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
+EXPORT_SYMBOL(uuid_be_index);
+
 /***************************************************************
  * Random UUID interface
  *
@@ -65,3 +72,66 @@ void uuid_be_gen(uuid_be *bu)
 	bu->b[6] = (bu->b[6] & 0x0F) | 0x40;
 }
 EXPORT_SYMBOL_GPL(uuid_be_gen);
+
+/**
+  * uuid_is_valid - checks if UUID string valid
+  * @uuid:	UUID string to check
+  *
+  * Description:
+  * It checks if the UUID string is following the format:
+  *	xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+  * where x is a hex digit.
+  *
+  * Return: 0 on success, %-EINVAL otherwise.
+  */
+int uuid_is_valid(const char *uuid)
+{
+	unsigned int i;
+
+	if (strnlen(uuid, UUID_STRING_LEN) < UUID_STRING_LEN)
+		return -EINVAL;
+
+	for (i = 0; i < UUID_STRING_LEN; i++) {
+		if (i == 8 || i == 13 || i == 18 || i == 23) {
+			if (uuid[i] != '-')
+				return -EINVAL;
+		} else if (!isxdigit(uuid[i])) {
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(uuid_is_valid);
+
+static int __uuid_to_bin(const char *uuid, __u8 b[16], const u8 ei[16])
+{
+	static const u8 si[16] = {0,2,4,6,9,11,14,16,19,21,24,26,28,30,32,34};
+	unsigned int i;
+	int ret;
+
+	ret = uuid_is_valid(uuid);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < 16; i++) {
+		int hi = hex_to_bin(uuid[si[i]] + 0);
+		int lo = hex_to_bin(uuid[si[i]] + 1);
+
+		b[ei[i]] = (hi << 4) | lo;
+	}
+
+	return 0;
+}
+
+int uuid_le_to_bin(const char *uuid, uuid_le *u)
+{
+	return __uuid_to_bin(uuid, u->b, uuid_le_index);
+}
+EXPORT_SYMBOL(uuid_le_to_bin);
+
+int uuid_be_to_bin(const char *uuid, uuid_be *u)
+{
+	return __uuid_to_bin(uuid, u->b, uuid_be_index);
+}
+EXPORT_SYMBOL(uuid_be_to_bin);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index be0e7cf..0967771 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -30,6 +30,7 @@
 #include <linux/ioport.h>
 #include <linux/dcache.h>
 #include <linux/cred.h>
+#include <linux/uuid.h>
 #include <net/addrconf.h>
 #ifdef CONFIG_BLOCK
 #include <linux/blkdev.h>
@@ -1304,19 +1305,17 @@ static noinline_for_stack
 char *uuid_string(char *buf, char *end, const u8 *addr,
 		  struct printf_spec spec, const char *fmt)
 {
-	char uuid[sizeof("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx")];
+	char uuid[UUID_STRING_LEN + 1];
 	char *p = uuid;
 	int i;
-	static const u8 be[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
-	static const u8 le[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
-	const u8 *index = be;
+	const u8 *index = uuid_be_index;
 	bool uc = false;
 
 	switch (*(++fmt)) {
 	case 'L':
 		uc = true;		/* fall-through */
 	case 'l':
-		index = le;
+		index = uuid_le_index;
 		break;
 	case 'B':
 		uc = true;
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 4/8] lib/uuid: remove FSF address
@ 2016-04-04 13:30   ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel, linux-efi, linux-api
  Cc: Andy Shevchenko

There is no point to keep an address in the file since it's a subject to
change.

While here, update Intel Copyright years.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/uuid.h      | 6 +-----
 include/uapi/linux/uuid.h | 4 ----
 lib/uuid.c                | 6 +-----
 3 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 616347f..115d04b 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -1,7 +1,7 @@
 /*
  * UUID/GUID definition
  *
- * Copyright (C) 2010, Intel Corp.
+ * Copyright (C) 2010, 2016 Intel Corp.
  *	Huang Ying <ying.huang@intel.com>
  *
  * This program is free software; you can redistribute it and/or
@@ -12,10 +12,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #ifndef _LINUX_UUID_H_
 #define _LINUX_UUID_H_
diff --git a/include/uapi/linux/uuid.h b/include/uapi/linux/uuid.h
index 786f077..3738e5f 100644
--- a/include/uapi/linux/uuid.h
+++ b/include/uapi/linux/uuid.h
@@ -12,10 +12,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
 #ifndef _UAPI_LINUX_UUID_H_
diff --git a/lib/uuid.c b/lib/uuid.c
index 995865c..0918232 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -1,7 +1,7 @@
 /*
  * Unified UUID/GUID definition
  *
- * Copyright (C) 2009, Intel Corp.
+ * Copyright (C) 2009, 2015 Intel Corp.
  *	Huang Ying <ying.huang@intel.com>
  *
  * This program is free software; you can redistribute it and/or
@@ -12,10 +12,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
 #include <linux/kernel.h>
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 4/8] lib/uuid: remove FSF address
@ 2016-04-04 13:30   ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
  Cc: Andy Shevchenko

There is no point to keep an address in the file since it's a subject to
change.

While here, update Intel Copyright years.

Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 include/linux/uuid.h      | 6 +-----
 include/uapi/linux/uuid.h | 4 ----
 lib/uuid.c                | 6 +-----
 3 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 616347f..115d04b 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -1,7 +1,7 @@
 /*
  * UUID/GUID definition
  *
- * Copyright (C) 2010, Intel Corp.
+ * Copyright (C) 2010, 2016 Intel Corp.
  *	Huang Ying <ying.huang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  *
  * This program is free software; you can redistribute it and/or
@@ -12,10 +12,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #ifndef _LINUX_UUID_H_
 #define _LINUX_UUID_H_
diff --git a/include/uapi/linux/uuid.h b/include/uapi/linux/uuid.h
index 786f077..3738e5f 100644
--- a/include/uapi/linux/uuid.h
+++ b/include/uapi/linux/uuid.h
@@ -12,10 +12,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
 #ifndef _UAPI_LINUX_UUID_H_
diff --git a/lib/uuid.c b/lib/uuid.c
index 995865c..0918232 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -1,7 +1,7 @@
 /*
  * Unified UUID/GUID definition
  *
- * Copyright (C) 2009, Intel Corp.
+ * Copyright (C) 2009, 2015 Intel Corp.
  *	Huang Ying <ying.huang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  *
  * This program is free software; you can redistribute it and/or
@@ -12,10 +12,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
 #include <linux/kernel.h>
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 5/8] sysctl: drop away useless label
@ 2016-04-04 13:30   ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel, linux-efi, linux-api
  Cc: Andy Shevchenko

We have no locking in bin_uuid(). Thus, we may remove the out label and use
return statements directly.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 kernel/sysctl_binary.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index 10a1d7d..055ad65 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -1123,15 +1123,14 @@ static ssize_t bin_uuid(struct file *file,
 
 		result = kernel_read(file, 0, buf, sizeof(buf) - 1);
 		if (result < 0)
-			goto out;
+			return result;
 
 		buf[result] = '\0';
 
 		/* Convert the uuid to from a string to binary */
 		for (i = 0; i < 16; i++) {
-			result = -EIO;
 			if (!isxdigit(str[0]) || !isxdigit(str[1]))
-				goto out;
+				return -EIO;
 
 			uuid[i] = (hex_to_bin(str[0]) << 4) |
 					hex_to_bin(str[1]);
@@ -1143,15 +1142,12 @@ static ssize_t bin_uuid(struct file *file,
 		if (oldlen > 16)
 			oldlen = 16;
 
-		result = -EFAULT;
 		if (copy_to_user(oldval, uuid, oldlen))
-			goto out;
+			return -EFAULT;
 
 		copied = oldlen;
 	}
-	result = copied;
-out:
-	return result;
+	return copied;
 }
 
 static ssize_t bin_dn_node_address(struct file *file,
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 5/8] sysctl: drop away useless label
@ 2016-04-04 13:30   ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
  Cc: Andy Shevchenko

We have no locking in bin_uuid(). Thus, we may remove the out label and use
return statements directly.

Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 kernel/sysctl_binary.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index 10a1d7d..055ad65 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -1123,15 +1123,14 @@ static ssize_t bin_uuid(struct file *file,
 
 		result = kernel_read(file, 0, buf, sizeof(buf) - 1);
 		if (result < 0)
-			goto out;
+			return result;
 
 		buf[result] = '\0';
 
 		/* Convert the uuid to from a string to binary */
 		for (i = 0; i < 16; i++) {
-			result = -EIO;
 			if (!isxdigit(str[0]) || !isxdigit(str[1]))
-				goto out;
+				return -EIO;
 
 			uuid[i] = (hex_to_bin(str[0]) << 4) |
 					hex_to_bin(str[1]);
@@ -1143,15 +1142,12 @@ static ssize_t bin_uuid(struct file *file,
 		if (oldlen > 16)
 			oldlen = 16;
 
-		result = -EFAULT;
 		if (copy_to_user(oldval, uuid, oldlen))
-			goto out;
+			return -EFAULT;
 
 		copied = oldlen;
 	}
-	result = copied;
-out:
-	return result;
+	return copied;
 }
 
 static ssize_t bin_dn_node_address(struct file *file,
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 6/8] sysctl: use generic UUID library
  2016-04-04 13:30 ` Andy Shevchenko
                   ` (5 preceding siblings ...)
  (?)
@ 2016-04-04 13:30 ` Andy Shevchenko
  -1 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel, linux-efi, linux-api
  Cc: Andy Shevchenko

UUID library provides uuid_be type and uuid_be_to_bin() function. This
substitutes open coded variant by generic library calls.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 kernel/sysctl_binary.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index 055ad65..ce94c45 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -13,6 +13,7 @@
 #include <linux/ctype.h>
 #include <linux/netdevice.h>
 #include <linux/kernel.h>
+#include <linux/uuid.h>
 #include <linux/slab.h>
 #include <linux/compat.h>
 
@@ -1117,9 +1118,8 @@ static ssize_t bin_uuid(struct file *file,
 
 	/* Only supports reads */
 	if (oldval && oldlen) {
-		char buf[40], *str = buf;
-		unsigned char uuid[16];
-		int i;
+		char buf[UUID_STRING_LEN + 1];
+		uuid_be uuid;
 
 		result = kernel_read(file, 0, buf, sizeof(buf) - 1);
 		if (result < 0)
@@ -1128,21 +1128,13 @@ static ssize_t bin_uuid(struct file *file,
 		buf[result] = '\0';
 
 		/* Convert the uuid to from a string to binary */
-		for (i = 0; i < 16; i++) {
-			if (!isxdigit(str[0]) || !isxdigit(str[1]))
-				return -EIO;
-
-			uuid[i] = (hex_to_bin(str[0]) << 4) |
-					hex_to_bin(str[1]);
-			str += 2;
-			if (*str == '-')
-				str++;
-		}
+		if (uuid_be_to_bin(buf, &uuid))
+			return -EIO;
 
 		if (oldlen > 16)
 			oldlen = 16;
 
-		if (copy_to_user(oldval, uuid, oldlen))
+		if (copy_to_user(oldval, &uuid, oldlen))
 			return -EFAULT;
 
 		copied = oldlen;
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 7/8] efi: redefine type, constant, macro from generic code
  2016-04-04 13:30 ` Andy Shevchenko
                   ` (6 preceding siblings ...)
  (?)
@ 2016-04-04 13:30 ` Andy Shevchenko
  -1 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel, linux-efi, linux-api
  Cc: Andy Shevchenko

Generic UUID library defines structure type, macro to define UUID, and the
length of the UUID string. This patch removes duplicate data structure
definition, UUID string length constant  as well as macro for UUID handling.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/efi.h | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 1626474..5b1d5c5 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -21,6 +21,7 @@
 #include <linux/pfn.h>
 #include <linux/pstore.h>
 #include <linux/reboot.h>
+#include <linux/uuid.h>
 
 #include <asm/page.h>
 
@@ -43,17 +44,10 @@ typedef u16 efi_char16_t;		/* UNICODE character */
 typedef u64 efi_physical_addr_t;
 typedef void *efi_handle_t;
 
-
-typedef struct {
-	u8 b[16];
-} efi_guid_t;
+typedef uuid_le efi_guid_t;
 
 #define EFI_GUID(a,b,c,d0,d1,d2,d3,d4,d5,d6,d7) \
-((efi_guid_t) \
-{{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
-  (b) & 0xff, ((b) >> 8) & 0xff, \
-  (c) & 0xff, ((c) >> 8) & 0xff, \
-  (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
+	UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)
 
 /*
  * Generic EFI table header
@@ -1050,7 +1044,7 @@ efi_reboot(enum reboot_mode reboot_mode, const char *__unused) {}
  * Length of a GUID string (strlen("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"))
  * not including trailing NUL
  */
-#define EFI_VARIABLE_GUID_LEN 36
+#define EFI_VARIABLE_GUID_LEN	UUID_STRING_LEN
 
 /*
  * The type of search to perform when calling boottime->locate_handle
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 8/8] efivars: use generic UUID library
  2016-04-04 13:30 ` Andy Shevchenko
                   ` (7 preceding siblings ...)
  (?)
@ 2016-04-04 13:30 ` Andy Shevchenko
  -1 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-04 13:30 UTC (permalink / raw)
  To: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Andrew Morton,
	Rasmus Villemoes, linux-kernel, linux-efi, linux-api
  Cc: Andy Shevchenko

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

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 fs/efivarfs/inode.c | 40 +++-------------------------------------
 1 file changed, 3 insertions(+), 37 deletions(-)

diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
index e2ab6d0..7103453 100644
--- a/fs/efivarfs/inode.c
+++ b/fs/efivarfs/inode.c
@@ -11,6 +11,7 @@
 #include <linux/fs.h>
 #include <linux/ctype.h>
 #include <linux/slab.h>
+#include <linux/uuid.h>
 
 #include "internal.h"
 
@@ -46,11 +47,7 @@ struct inode *efivarfs_get_inode(struct super_block *sb,
  */
 bool efivarfs_valid_name(const char *str, int len)
 {
-	static const char dashes[EFI_VARIABLE_GUID_LEN] = {
-		[8] = 1, [13] = 1, [18] = 1, [23] = 1
-	};
 	const char *s = str + len - EFI_VARIABLE_GUID_LEN;
-	int i;
 
 	/*
 	 * We need a GUID, plus at least one letter for the variable name,
@@ -68,37 +65,7 @@ bool efivarfs_valid_name(const char *str, int len)
 	 *
 	 *	12345678-1234-1234-1234-123456789abc
 	 */
-	for (i = 0; i < EFI_VARIABLE_GUID_LEN; i++) {
-		if (dashes[i]) {
-			if (*s++ != '-')
-				return false;
-		} else {
-			if (!isxdigit(*s++))
-				return false;
-		}
-	}
-
-	return true;
-}
-
-static void efivarfs_hex_to_guid(const char *str, efi_guid_t *guid)
-{
-	guid->b[0] = hex_to_bin(str[6]) << 4 | hex_to_bin(str[7]);
-	guid->b[1] = hex_to_bin(str[4]) << 4 | hex_to_bin(str[5]);
-	guid->b[2] = hex_to_bin(str[2]) << 4 | hex_to_bin(str[3]);
-	guid->b[3] = hex_to_bin(str[0]) << 4 | hex_to_bin(str[1]);
-	guid->b[4] = hex_to_bin(str[11]) << 4 | hex_to_bin(str[12]);
-	guid->b[5] = hex_to_bin(str[9]) << 4 | hex_to_bin(str[10]);
-	guid->b[6] = hex_to_bin(str[16]) << 4 | hex_to_bin(str[17]);
-	guid->b[7] = hex_to_bin(str[14]) << 4 | hex_to_bin(str[15]);
-	guid->b[8] = hex_to_bin(str[19]) << 4 | hex_to_bin(str[20]);
-	guid->b[9] = hex_to_bin(str[21]) << 4 | hex_to_bin(str[22]);
-	guid->b[10] = hex_to_bin(str[24]) << 4 | hex_to_bin(str[25]);
-	guid->b[11] = hex_to_bin(str[26]) << 4 | hex_to_bin(str[27]);
-	guid->b[12] = hex_to_bin(str[28]) << 4 | hex_to_bin(str[29]);
-	guid->b[13] = hex_to_bin(str[30]) << 4 | hex_to_bin(str[31]);
-	guid->b[14] = hex_to_bin(str[32]) << 4 | hex_to_bin(str[33]);
-	guid->b[15] = hex_to_bin(str[34]) << 4 | hex_to_bin(str[35]);
+	return uuid_is_valid(s) == 0;
 }
 
 static int efivarfs_create(struct inode *dir, struct dentry *dentry,
@@ -119,8 +86,7 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry,
 	/* length of the variable name itself: remove GUID and separator */
 	namelen = dentry->d_name.len - EFI_VARIABLE_GUID_LEN - 1;
 
-	efivarfs_hex_to_guid(dentry->d_name.name + namelen + 1,
-			&var->var.VendorGuid);
+	uuid_le_to_bin(dentry->d_name.name + namelen + 1, &var->var.VendorGuid);
 
 	if (efivar_variable_is_removable(var->var.VendorGuid,
 					 dentry->d_name.name, namelen))
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 0/8] uuid: convert users to generic UUID API
  2016-04-04 13:30 ` Andy Shevchenko
                   ` (8 preceding siblings ...)
  (?)
@ 2016-04-04 23:40 ` Andrew Morton
  2016-04-05 11:07     ` Andy Shevchenko
  2016-04-05 14:06     ` Matt Fleming
  -1 siblings, 2 replies; 23+ messages in thread
From: Andrew Morton @ 2016-04-04 23:40 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Rasmus Villemoes,
	linux-kernel, linux-efi, linux-api

On Mon,  4 Apr 2016 16:30:02 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> There are few functions here and there along with type definitions that provide
> UUID API. This series consolidates everything under one hood and converts
> current users.

Well.  It converts two current users.

There are many references to "uuid" in the tree.  How many sites could
potentially benefit from this change?

> This has been tested for a while internally, however it doesn't mean we covered
> all possible cases (especially accuracy of UUID constants after conversion).
> So, please test this as much as you can and provide your tag. We appreciate the
> effort.

I'm not sure who this series was aimed at but I saw "lib" and grabbed
it.  I'll await Matt's ack.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 3/8] lib/uuid: introduce few more generic helpers for UUID
  2016-04-04 13:30 ` [PATCH v2 3/8] lib/uuid: introduce few more generic helpers for UUID Andy Shevchenko
@ 2016-04-04 23:40   ` Andrew Morton
  2016-04-04 23:55     ` Joe Perches
                       ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Andrew Morton @ 2016-04-04 23:40 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Rasmus Villemoes,
	linux-kernel, linux-efi, linux-api

On Mon,  4 Apr 2016 16:30:05 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> There are new helpers in this patch:
> 
> uuid_is_valid		checks if a UUID is valid
> uuid_be_to_bin		converts from string to binary (big endian)
> uuid_le_to_bin		converts from string to binary (little endian)
	> 
> They will be used in future, i.e. in the following patches in the series.
> 
> This also moves indices arrays to lib/uuid.c to be shared accross modules.
> 
> ...
>
> --- a/include/linux/uuid.h
> +++ b/include/linux/uuid.h

Nit:

> +/**
> +  * uuid_is_valid - checks if UUID string valid
> +  * @uuid:	UUID string to check
> +  *
> +  * Description:
> +  * It checks if the UUID string is following the format:
> +  *	xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> +  * where x is a hex digit.
> +  *
> +  * Return: 0 on success, %-EINVAL otherwise.
> +  */
> +int uuid_is_valid(const char *uuid)
> +{
> +	unsigned int i;
> +
> +	if (strnlen(uuid, UUID_STRING_LEN) < UUID_STRING_LEN)
> +		return -EINVAL;
> +
> +	for (i = 0; i < UUID_STRING_LEN; i++) {
> +		if (i == 8 || i == 13 || i == 18 || i == 23) {
> +			if (uuid[i] != '-')
> +				return -EINVAL;
> +		} else if (!isxdigit(uuid[i])) {
> +			return -EINVAL;
> +		}
> +	}

Could add

	if (uuid[i])
		return -EINVAL;

here and lose the additional pass across the input (strlen).

> +	return 0;
> +}
>
> ...
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 3/8] lib/uuid: introduce few more generic helpers for UUID
  2016-04-04 23:40   ` Andrew Morton
@ 2016-04-04 23:55     ` Joe Perches
  2016-04-05  0:48       ` Stephen Rothwell
  2016-04-05 10:51       ` Andy Shevchenko
  2 siblings, 0 replies; 23+ messages in thread
From: Joe Perches @ 2016-04-04 23:55 UTC (permalink / raw)
  To: Andrew Morton, Andy Shevchenko
  Cc: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Rasmus Villemoes,
	linux-kernel, linux-efi, linux-api

On Mon, 2016-04-04 at 16:40 -0700, Andrew Morton wrote:
> On Mon,  4 Apr 2016 16:30:05 +0300 Andy Shevchenko  wrote:
> 
> > 
> > There are new helpers in this patch:
> > 
> > uuid_is_valid		checks if a UUID is valid
> > uuid_be_to_bin		converts from string to binary (big endian)
> > uuid_le_to_bin		converts from string to binary (little endian)
> 	> 
> > 
> > They will be used in future, i.e. in the following patches in the series.
> > 
> > This also moves indices arrays to lib/uuid.c to be shared accross modules.
> > 
> > ...
> > 
> > --- a/include/linux/uuid.h
> > +++ b/include/linux/uuid.h
> Nit:
> 
> > 
> > +/**
> > +  * uuid_is_valid - checks if UUID string valid
> > +  * @uuid:	UUID string to check
> > +  *
> > +  * Description:
> > +  * It checks if the UUID string is following the format:
> > +  *	xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> > +  * where x is a hex digit.
> > +  *
> > +  * Return: 0 on success, %-EINVAL otherwise.
> > +  */
> > +int uuid_is_valid(const char *uuid)
> > +{
> > +	unsigned int i;
> > +
> > +	if (strnlen(uuid, UUID_STRING_LEN) < UUID_STRING_LEN)
> > +		return -EINVAL;
> > +
> > +	for (i = 0; i < UUID_STRING_LEN; i++) {
> > +		if (i == 8 || i == 13 || i == 18 || i == 23) {
> > +			if (uuid[i] != '-')
> > +				return -EINVAL;
> > +		} else if (!isxdigit(uuid[i])) {
> > +			return -EINVAL;
> > +		}
> > +	}
> Could add
> 
> 	if (uuid[i])
> 		return -EINVAL;
> 
> here and lose the additional pass across the input (strlen).

nit2:

Could make this return bool.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 3/8] lib/uuid: introduce few more generic helpers for UUID
@ 2016-04-05  0:48       ` Stephen Rothwell
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Rothwell @ 2016-04-05  0:48 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andrew Morton, Arnd Bergmann, Theodore Ts'o, Matt Fleming,
	Rasmus Villemoes, linux-kernel, linux-efi, linux-api

Hi Andy,

On Mon, 4 Apr 2016 16:40:29 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Mon,  4 Apr 2016 16:30:05 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > There are new helpers in this patch:
> > 
> > uuid_is_valid		checks if a UUID is valid
> > uuid_be_to_bin		converts from string to binary (big endian)
> > uuid_le_to_bin		converts from string to binary (little endian)
> 	> 
> > They will be used in future, i.e. in the following patches in the series.
> > 
> > This also moves indices arrays to lib/uuid.c to be shared accross modules.
> > 
> > ...
> >
> > --- a/include/linux/uuid.h
> > +++ b/include/linux/uuid.h  
> 
> Nit:
> 
> > +/**
> > +  * uuid_is_valid - checks if UUID string valid
> > +  * @uuid:	UUID string to check
> > +  *
> > +  * Description:
> > +  * It checks if the UUID string is following the format:
> > +  *	xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> > +  * where x is a hex digit.
> > +  *
> > +  * Return: 0 on success, %-EINVAL otherwise.
> > +  */
> > +int uuid_is_valid(const char *uuid)
> > +{
> > +	unsigned int i;
> > +
> > +	if (strnlen(uuid, UUID_STRING_LEN) < UUID_STRING_LEN)
> > +		return -EINVAL;
> > +
> > +	for (i = 0; i < UUID_STRING_LEN; i++) {
> > +		if (i == 8 || i == 13 || i == 18 || i == 23) {
> > +			if (uuid[i] != '-')
> > +				return -EINVAL;
> > +		} else if (!isxdigit(uuid[i])) {
> > +			return -EINVAL;
> > +		}
> > +	}  
> 
> Could add
> 
> 	if (uuid[i])
> 		return -EINVAL;
> 
> here and lose the additional pass across the input (strlen).

also, why is this not a bool function?

-- 
Cheers,
Stephen Rothwell

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 3/8] lib/uuid: introduce few more generic helpers for UUID
@ 2016-04-05  0:48       ` Stephen Rothwell
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Rothwell @ 2016-04-05  0:48 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andrew Morton, Arnd Bergmann, Theodore Ts'o, Matt Fleming,
	Rasmus Villemoes, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA

Hi Andy,

On Mon, 4 Apr 2016 16:40:29 -0700 Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
>
> On Mon,  4 Apr 2016 16:30:05 +0300 Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> 
> > There are new helpers in this patch:
> > 
> > uuid_is_valid		checks if a UUID is valid
> > uuid_be_to_bin		converts from string to binary (big endian)
> > uuid_le_to_bin		converts from string to binary (little endian)
> 	> 
> > They will be used in future, i.e. in the following patches in the series.
> > 
> > This also moves indices arrays to lib/uuid.c to be shared accross modules.
> > 
> > ...
> >
> > --- a/include/linux/uuid.h
> > +++ b/include/linux/uuid.h  
> 
> Nit:
> 
> > +/**
> > +  * uuid_is_valid - checks if UUID string valid
> > +  * @uuid:	UUID string to check
> > +  *
> > +  * Description:
> > +  * It checks if the UUID string is following the format:
> > +  *	xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> > +  * where x is a hex digit.
> > +  *
> > +  * Return: 0 on success, %-EINVAL otherwise.
> > +  */
> > +int uuid_is_valid(const char *uuid)
> > +{
> > +	unsigned int i;
> > +
> > +	if (strnlen(uuid, UUID_STRING_LEN) < UUID_STRING_LEN)
> > +		return -EINVAL;
> > +
> > +	for (i = 0; i < UUID_STRING_LEN; i++) {
> > +		if (i == 8 || i == 13 || i == 18 || i == 23) {
> > +			if (uuid[i] != '-')
> > +				return -EINVAL;
> > +		} else if (!isxdigit(uuid[i])) {
> > +			return -EINVAL;
> > +		}
> > +	}  
> 
> Could add
> 
> 	if (uuid[i])
> 		return -EINVAL;
> 
> here and lose the additional pass across the input (strlen).

also, why is this not a bool function?

-- 
Cheers,
Stephen Rothwell

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 3/8] lib/uuid: introduce few more generic helpers for UUID
@ 2016-04-05 10:51       ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-05 10:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Rasmus Villemoes,
	linux-kernel, linux-efi, linux-api

On Mon, 2016-04-04 at 16:40 -0700, Andrew Morton wrote:
> On Mon,  4 Apr 2016 16:30:05 +0300 Andy Shevchenko <andriy.shevchenko
> @linux.intel.com> wrote:
> 
> > 
> > There are new helpers in this patch:
> > 
> > uuid_is_valid		checks if a UUID is valid
> > uuid_be_to_bin		converts from string to binary (big
> > endian)
> > uuid_le_to_bin		converts from string to binary
> > (little endian)
> 	> 
> > 
> > They will be used in future, i.e. in the following patches in the
> > series.
> > 
> > This also moves indices arrays to lib/uuid.c to be shared accross
> > modules.
> > 
> > ...
> > 
> > --- a/include/linux/uuid.h
> > +++ b/include/linux/uuid.h
> Nit:
> 
> > 
> > +/**
> > +  * uuid_is_valid - checks if UUID string valid
> > +  * @uuid:	UUID string to check
> > +  *
> > +  * Description:
> > +  * It checks if the UUID string is following the format:
> > +  *	xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> > +  * where x is a hex digit.
> > +  *
> > +  * Return: 0 on success, %-EINVAL otherwise.
> > +  */
> > +int uuid_is_valid(const char *uuid)
> > +{
> > +	unsigned int i;
> > +
> > +	if (strnlen(uuid, UUID_STRING_LEN) < UUID_STRING_LEN)
> > +		return -EINVAL;
> > +
> > +	for (i = 0; i < UUID_STRING_LEN; i++) {
> > +		if (i == 8 || i == 13 || i == 18 || i == 23) {
> > +			if (uuid[i] != '-')
> > +				return -EINVAL;
> > +		} else if (!isxdigit(uuid[i])) {
> > +			return -EINVAL;
> > +		}
> > +	}
> Could add
> 
> 	if (uuid[i])
> 		return -EINVAL;
> 
> here and lose the additional pass across the input (strlen).

Others suggested a boolean. I would send a fixup later.

> 
> > 
> > +	return 0;
> > +}
> > 
> > ...
> > 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 3/8] lib/uuid: introduce few more generic helpers for UUID
@ 2016-04-05 10:51       ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-05 10:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Rasmus Villemoes,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA

On Mon, 2016-04-04 at 16:40 -0700, Andrew Morton wrote:
> On Mon,  4 Apr 2016 16:30:05 +0300 Andy Shevchenko <andriy.shevchenko
> @linux.intel.com> wrote:
> 
> > 
> > There are new helpers in this patch:
> > 
> > uuid_is_valid		checks if a UUID is valid
> > uuid_be_to_bin		converts from string to binary (big
> > endian)
> > uuid_le_to_bin		converts from string to binary
> > (little endian)
> 	> 
> > 
> > They will be used in future, i.e. in the following patches in the
> > series.
> > 
> > This also moves indices arrays to lib/uuid.c to be shared accross
> > modules.
> > 
> > ...
> > 
> > --- a/include/linux/uuid.h
> > +++ b/include/linux/uuid.h
> Nit:
> 
> > 
> > +/**
> > +  * uuid_is_valid - checks if UUID string valid
> > +  * @uuid:	UUID string to check
> > +  *
> > +  * Description:
> > +  * It checks if the UUID string is following the format:
> > +  *	xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> > +  * where x is a hex digit.
> > +  *
> > +  * Return: 0 on success, %-EINVAL otherwise.
> > +  */
> > +int uuid_is_valid(const char *uuid)
> > +{
> > +	unsigned int i;
> > +
> > +	if (strnlen(uuid, UUID_STRING_LEN) < UUID_STRING_LEN)
> > +		return -EINVAL;
> > +
> > +	for (i = 0; i < UUID_STRING_LEN; i++) {
> > +		if (i == 8 || i == 13 || i == 18 || i == 23) {
> > +			if (uuid[i] != '-')
> > +				return -EINVAL;
> > +		} else if (!isxdigit(uuid[i])) {
> > +			return -EINVAL;
> > +		}
> > +	}
> Could add
> 
> 	if (uuid[i])
> 		return -EINVAL;
> 
> here and lose the additional pass across the input (strlen).

Others suggested a boolean. I would send a fixup later.

> 
> > 
> > +	return 0;
> > +}
> > 
> > ...
> > 

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 0/8] uuid: convert users to generic UUID API
@ 2016-04-05 11:07     ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-05 11:07 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Rasmus Villemoes,
	linux-kernel, linux-efi, linux-api

On Mon, 2016-04-04 at 16:40 -0700, Andrew Morton wrote:
> On Mon,  4 Apr 2016 16:30:02 +0300 Andy Shevchenko <andriy.shevchenko
> @linux.intel.com> wrote:
> 
> > 
> > There are few functions here and there along with type definitions
> > that provide
> > UUID API. This series consolidates everything under one hood and
> > converts
> > current users.
> Well.  It converts two current users.
> 
> There are many references to "uuid" in the tree.  How many sites
> could
> potentially benefit from this change?

The big chunk is ACPI [1] though Rafael pointed out to some type
dereferencing he didn't get [2]. (By the way there is already use of
such in ACPI code from 2010: drivers/acpi/apei/ghes.c). That's why I
postponed that change.

[1] http://www.spinics.net/lists/linux-acpi/msg63811.html
[2] http://www.spinics.net/lists/linux-acpi/msg63854.html

> 
> > 
> > This has been tested for a while internally, however it doesn't
> > mean we covered
> > all possible cases (especially accuracy of UUID constants after
> > conversion).
> > So, please test this as much as you can and provide your tag. We
> > appreciate the
> > effort.
> I'm not sure who this series was aimed at but I saw "lib" and grabbed
> it.  I'll await Matt's ack.

Yes, this is lib/ stuff, I hope the rest would provide Ack.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 0/8] uuid: convert users to generic UUID API
@ 2016-04-05 11:07     ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-05 11:07 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Arnd Bergmann, Theodore Ts'o, Matt Fleming, Rasmus Villemoes,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA

On Mon, 2016-04-04 at 16:40 -0700, Andrew Morton wrote:
> On Mon,  4 Apr 2016 16:30:02 +0300 Andy Shevchenko <andriy.shevchenko
> @linux.intel.com> wrote:
> 
> > 
> > There are few functions here and there along with type definitions
> > that provide
> > UUID API. This series consolidates everything under one hood and
> > converts
> > current users.
> Well.  It converts two current users.
> 
> There are many references to "uuid" in the tree.  How many sites
> could
> potentially benefit from this change?

The big chunk is ACPI [1] though Rafael pointed out to some type
dereferencing he didn't get [2]. (By the way there is already use of
such in ACPI code from 2010: drivers/acpi/apei/ghes.c). That's why I
postponed that change.

[1] http://www.spinics.net/lists/linux-acpi/msg63811.html
[2] http://www.spinics.net/lists/linux-acpi/msg63854.html

> 
> > 
> > This has been tested for a while internally, however it doesn't
> > mean we covered
> > all possible cases (especially accuracy of UUID constants after
> > conversion).
> > So, please test this as much as you can and provide your tag. We
> > appreciate the
> > effort.
> I'm not sure who this series was aimed at but I saw "lib" and grabbed
> it.  I'll await Matt's ack.

Yes, this is lib/ stuff, I hope the rest would provide Ack.

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 0/8] uuid: convert users to generic UUID API
@ 2016-04-05 14:06     ` Matt Fleming
  0 siblings, 0 replies; 23+ messages in thread
From: Matt Fleming @ 2016-04-05 14:06 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andy Shevchenko, Arnd Bergmann, Theodore Ts'o,
	Rasmus Villemoes, linux-kernel, linux-efi, linux-api

On Mon, 04 Apr, at 04:40:20PM, Andrew Morton wrote:
> 
> I'm not sure who this series was aimed at but I saw "lib" and grabbed
> it.  I'll await Matt's ack.
 
Happy for you to take this series Andrew.

I saw Andy just sent out a v3. For the EFI bits,

Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 0/8] uuid: convert users to generic UUID API
@ 2016-04-05 14:06     ` Matt Fleming
  0 siblings, 0 replies; 23+ messages in thread
From: Matt Fleming @ 2016-04-05 14:06 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andy Shevchenko, Arnd Bergmann, Theodore Ts'o,
	Rasmus Villemoes, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA

On Mon, 04 Apr, at 04:40:20PM, Andrew Morton wrote:
> 
> I'm not sure who this series was aimed at but I saw "lib" and grabbed
> it.  I'll await Matt's ack.
 
Happy for you to take this series Andrew.

I saw Andy just sent out a v3. For the EFI bits,

Reviewed-by: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2016-04-05 14:09 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-04 13:30 [PATCH v2 0/8] uuid: convert users to generic UUID API Andy Shevchenko
2016-04-04 13:30 ` Andy Shevchenko
2016-04-04 13:30 ` [PATCH v2 1/8] lib/vsprintf: simplify UUID printing Andy Shevchenko
2016-04-04 13:30 ` [PATCH v2 2/8] lib/uuid: move generate_random_uuid() to uuid.c Andy Shevchenko
2016-04-04 13:30 ` [PATCH v2 3/8] lib/uuid: introduce few more generic helpers for UUID Andy Shevchenko
2016-04-04 23:40   ` Andrew Morton
2016-04-04 23:55     ` Joe Perches
2016-04-05  0:48     ` Stephen Rothwell
2016-04-05  0:48       ` Stephen Rothwell
2016-04-05 10:51     ` Andy Shevchenko
2016-04-05 10:51       ` Andy Shevchenko
2016-04-04 13:30 ` [PATCH v2 4/8] lib/uuid: remove FSF address Andy Shevchenko
2016-04-04 13:30   ` Andy Shevchenko
2016-04-04 13:30 ` [PATCH v2 5/8] sysctl: drop away useless label Andy Shevchenko
2016-04-04 13:30   ` Andy Shevchenko
2016-04-04 13:30 ` [PATCH v2 6/8] sysctl: use generic UUID library Andy Shevchenko
2016-04-04 13:30 ` [PATCH v2 7/8] efi: redefine type, constant, macro from generic code Andy Shevchenko
2016-04-04 13:30 ` [PATCH v2 8/8] efivars: use generic UUID library Andy Shevchenko
2016-04-04 23:40 ` [PATCH v2 0/8] uuid: convert users to generic UUID API Andrew Morton
2016-04-05 11:07   ` Andy Shevchenko
2016-04-05 11:07     ` Andy Shevchenko
2016-04-05 14:06   ` Matt Fleming
2016-04-05 14:06     ` Matt Fleming

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.