[PATCH 01/12] kstrto*: accept "-0" for signed conversion

[PATCH 01/12] kstrto*: accept "-0" for signed conversion

[Alexey Dobriyan]

strtol(3) et al accept "-0", so should we.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 lib/kstrtox.c      | 2 +-
 lib/test-kstrtox.c | 6 +-----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index ec8da78..94be244 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -152,7 +152,7 @@ int kstrtoll(const char *s, unsigned int base, long long *res)
 		rv = _kstrtoull(s + 1, base, &tmp);
 		if (rv < 0)
 			return rv;
-		if ((long long)(-tmp) >= 0)
+		if ((long long)-tmp > 0)
 			return -ERANGE;
 		*res = -tmp;
 	} else {
diff --git a/lib/test-kstrtox.c b/lib/test-kstrtox.c
index 4137bca..f355f67 100644
--- a/lib/test-kstrtox.c
+++ b/lib/test-kstrtox.c
@@ -260,6 +260,7 @@ static void __init test_kstrtoll_ok(void)
 		{"4294967297",	10,	4294967297LL},
 		{"9223372036854775807",	10,	9223372036854775807LL},
 
+		{"-0",	10,	0LL},
 		{"-1",	10,	-1LL},
 		{"-2",	10,	-2LL},
 		{"-9223372036854775808",	10,	LLONG_MIN},
@@ -277,11 +278,6 @@ static void __init test_kstrtoll_fail(void)
 		{"-9223372036854775809",	10},
 		{"-18446744073709551614",	10},
 		{"-18446744073709551615",	10},
-		/* negative zero isn't an integer in Linux */
-		{"-0",	0},
-		{"-0",	8},
-		{"-0",	10},
-		{"-0",	16},
 		/* sign is first character if any */
 		{"-+1", 0},
 		{"-+1", 8},

[PATCH 02/12] Add parse_integer() (replacement for simple_strto*())

[Alexey Dobriyan]

kstrto*() and kstrto*_from_user() family of functions were added
to help with parsing one integer written as string to proc/sysfs/debugfs
files. But they have a limitation: string passed must end with \0 or \n\0.
There are enough places where kstrto*() functions can't be used because of
this limitation. Trivial example: major:minor "%u:%u".

Currently the only way to parse everything is simple_strto*() functions.
But they are suboptimal:
* they do not detect overflow (can be fixed, but no one bothered since ~0.99.11),
* there are only 4 of them -- long and "long long" versions,
  This leads to silent truncation in the most simple case:

	val = strtoul(s, NULL, 0);

* half of the people think that "char **endp" argument is necessary and
  add unnecessary variable.

OpenBSD people, fed up with how complex correct integer parsing is, added
strtonum(3) to fixup for deficiencies of libc-style integer parsing:
http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/strtonum.3?query=strtonum&arch=i386

It'd be OK to copy that but it relies on "errno" and fixed strings as
error reporting channel which I think is not OK for kernel.
strtonum() also doesn't report number of characted consumed.

What to do?

Enter parse_integer().

	int parse_integer(const char *s, unsigned int base, T *val);

Rationale:
* parse_integer() is exactly 1 (one) interface not 4 or many, one for each type.

* parse_integer() reports -E errors reliably in a canonical kernel way:

	rv = parse_integer(str, 10, &val);
	if (rv < 0)
		return rv;

* parse_integer() writes result only if there were no errors,
  at least one digit has to be consumed,

* parse_integer doesn't mix error reporting channel and value channel,
  it does mix error and number-of-character-consumed channel, though.

* parse_integer() reports number of characters consumed, makes parsing
  multiple values easy:

	rv = parse_integer(str, 0, &val1);
	if (rv < 0)
		return rv;
	str += rv;
	if (*str++ != ':')
		return -EINVAL;
	rv = parse_integer(str, 0, &val2);
	if (rv < 0)
		return rv;
	if (str[rv] != '\0')
		return -EINVAL;

There are several deficiencies in parse_integer() compared to strto*():

* can't be used in initializers:

	const T x = strtoul();

* can't be used with bitfields,
* can't be used in expressions:

	x = strtol() * 1024;
	x = y = strtol();
	x += strtol();

* currently there is no support for _Bool and at least one place
  where simple_strtoul() is directly assigned to _Bool variable.
  It is trivial to add, but not clear if it should only accept "0" and "1",
  because, say, module param code accepts 0, 1, y, n, Y and N.

In defense of parse_integer() all I can say, is that using strtol() in
expression or initializer promotes no error checking and thus probably
should not be encouraged in C, language with no built-in error checking
anyway.

The amount of "x = y = strtol()" expressions in kernel is very small.
The amount of direct usage in expression is not small, but can be
counted as an acceptable loss.

Misc suggestions and ideas from Rasmus Villemoes.

v2:
* self-contained header
* trim code size a bit
* testsuite

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 include/linux/kernel.h        |   7 +-
 include/linux/parse-integer.h |  79 +++++++++++++++++
 lib/Makefile                  |   1 +
 lib/kstrtox.c                 |  76 +++++-----------
 lib/kstrtox.h                 |   1 -
 lib/parse-integer.c           | 198 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 305 insertions(+), 57 deletions(-)
 create mode 100644 include/linux/parse-integer.h
 create mode 100644 lib/parse-integer.c

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3a5b48e..9d3f543 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -10,6 +10,7 @@
 #include <linux/bitops.h>
 #include <linux/log2.h>
 #include <linux/typecheck.h>
+#include <linux/parse-integer.h>
 #include <linux/printk.h>
 #include <linux/dynamic_debug.h>
 #include <asm/byteorder.h>
@@ -387,8 +388,10 @@ static inline int __must_check kstrtos32_from_user(const char __user *s, size_t
 	return kstrtoint_from_user(s, count, base, res);
 }
 
-/* Obsolete, do not use.  Use kstrto<foo> instead */
-
+/*
+ * Obsolete, do not use.
+ * Use parse_integer(), kstrto*(), kstrto*_from_user(), sscanf().
+ */
 extern unsigned long simple_strtoul(const char *,char **,unsigned int);
 extern long simple_strtol(const char *,char **,unsigned int);
 extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
diff --git a/include/linux/parse-integer.h b/include/linux/parse-integer.h
new file mode 100644
index 0000000..5dd0347
--- /dev/null
+++ b/include/linux/parse-integer.h
@@ -0,0 +1,79 @@
+#ifndef _PARSE_INTEGER_H
+#define _PARSE_INTEGER_H
+#include <linux/compiler.h>
+#include <linux/types.h>
+
+/*
+ * int parse_integer(const char *s, unsigned int base, T *val);
+ *
+ * Convert integer string representation to an integer.
+ * Range of accepted values equals to that of type T.
+ *
+ * Conversion to unsigned integer accepts sign "+".
+ * Conversion to signed integer accepts sign "+" and sign "-".
+ *
+ * Radix 0 means autodetection: leading "0x" implies radix 16,
+ * leading "0" implies radix 8, otherwise radix is 10.
+ * Autodetection hint works after optional sign, but not before.
+ *
+ * Return number of characters parsed or -E.
+ *
+ * "T=char" case is not supported because -f{un,}signed-char can silently
+ * change range of accepted values.
+ */
+#define parse_integer(s, base, val)	\
+({					\
+	const char *_s = (s);		\
+	unsigned int _base = (base);	\
+	typeof(&(val)[0]) _val = (val);	\
+					\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), signed char *),	\
+	_parse_integer_sc(_s, _base, (void *)_val),			\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), unsigned char *),	\
+	_parse_integer_uc(_s, _base, (void *)_val),			\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), short *),		\
+	_parse_integer_s(_s, _base, (void *)_val),			\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), unsigned short *),	\
+	_parse_integer_us(_s, _base, (void *)_val),			\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), int *),		\
+	_parse_integer_i(_s, _base, (void *)_val),			\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), unsigned int *),	\
+	_parse_integer_u(_s, _base, (void *)_val),			\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), long *) && sizeof(long) == 4,\
+	_parse_integer_i(_s, _base, (void *)_val),			\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), long *) && sizeof(long) == 8,\
+	_parse_integer_ll(_s, _base, (void *)_val),			\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), unsigned long *) && sizeof(unsigned long) == 4,\
+	_parse_integer_u(_s, _base, (void *)_val),			\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), unsigned long *) && sizeof(unsigned long) == 8,\
+	_parse_integer_ull(_s, _base, (void *)_val),			\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), long long *),	\
+	_parse_integer_ll(_s, _base, (void *)_val),			\
+	__builtin_choose_expr(						\
+	__builtin_types_compatible_p(typeof(_val), unsigned long long *),\
+	_parse_integer_ull(_s, _base, (void *)_val),			\
+	_parse_integer_link_time_error()))))))))))));	\
+})
+/* internal, do not use */
+int _parse_integer_sc(const char *s, unsigned int base, signed char *val);
+int _parse_integer_uc(const char *s, unsigned int base, unsigned char *val);
+int _parse_integer_s(const char *s, unsigned int base, short *val);
+int _parse_integer_us(const char *s, unsigned int base, unsigned short *val);
+int _parse_integer_i(const char *s, unsigned int base, int *val);
+int _parse_integer_u(const char *s, unsigned int base, unsigned int *val);
+int _parse_integer_ll(const char *s, unsigned int base, long long *val);
+int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val);
+void _parse_integer_link_time_error(void);
+const char *_parse_integer_fixup_radix(const char *s, unsigned int *base);
+#endif
diff --git a/lib/Makefile b/lib/Makefile
index 6c37933..e69b356 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
 obj-y += hexdump.o
 obj-$(CONFIG_TEST_HEXDUMP) += test-hexdump.o
 obj-y += kstrtox.o
+obj-y += parse-integer.o
 obj-$(CONFIG_TEST_BPF) += test_bpf.o
 obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o
 obj-$(CONFIG_TEST_KASAN) += test_kasan.o
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index 94be244..652fd6b 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -20,22 +20,6 @@
 #include <asm/uaccess.h>
 #include "kstrtox.h"
 
-const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
-{
-	if (*base == 0) {
-		if (s[0] == '0') {
-			if (_tolower(s[1]) == 'x' && isxdigit(s[2]))
-				*base = 16;
-			else
-				*base = 8;
-		} else
-			*base = 10;
-	}
-	if (*base == 16 && s[0] == '0' && _tolower(s[1]) == 'x')
-		s += 2;
-	return s;
-}
-
 /*
  * Convert non-negative integer string representation in explicitly given radix
  * to an integer.
@@ -83,26 +67,6 @@ unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long
 	return rv;
 }
 
-static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
-{
-	unsigned long long _res;
-	unsigned int rv;
-
-	s = _parse_integer_fixup_radix(s, &base);
-	rv = _parse_integer(s, base, &_res);
-	if (rv & KSTRTOX_OVERFLOW)
-		return -ERANGE;
-	if (rv == 0)
-		return -EINVAL;
-	s += rv;
-	if (*s == '\n')
-		s++;
-	if (*s)
-		return -EINVAL;
-	*res = _res;
-	return 0;
-}
-
 /**
  * kstrtoull - convert a string to an unsigned long long
  * @s: The start of the string. The string must be null-terminated, and may also
@@ -121,9 +85,19 @@ static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
  */
 int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
 {
-	if (s[0] == '+')
+	unsigned long long _res;
+	int rv;
+
+	rv = parse_integer(s, base, &_res);
+	if (rv < 0)
+		return rv;
+	s += rv;
+	if (*s == '\n')
 		s++;
-	return _kstrtoull(s, base, res);
+	if (*s)
+		return -EINVAL;
+	*res = _res;
+	return 0;
 }
 EXPORT_SYMBOL(kstrtoull);
 
@@ -145,24 +119,18 @@ EXPORT_SYMBOL(kstrtoull);
  */
 int kstrtoll(const char *s, unsigned int base, long long *res)
 {
-	unsigned long long tmp;
+	long long _res;
 	int rv;
 
-	if (s[0] == '-') {
-		rv = _kstrtoull(s + 1, base, &tmp);
-		if (rv < 0)
-			return rv;
-		if ((long long)-tmp > 0)
-			return -ERANGE;
-		*res = -tmp;
-	} else {
-		rv = kstrtoull(s, base, &tmp);
-		if (rv < 0)
-			return rv;
-		if ((long long)tmp < 0)
-			return -ERANGE;
-		*res = tmp;
-	}
+	rv = parse_integer(s, base, &_res);
+	if (rv < 0)
+		return rv;
+	s += rv;
+	if (*s == '\n')
+		s++;
+	if (*s)
+		return -EINVAL;
+	*res = _res;
 	return 0;
 }
 EXPORT_SYMBOL(kstrtoll);
diff --git a/lib/kstrtox.h b/lib/kstrtox.h
index f13eeea..7b1f447 100644
--- a/lib/kstrtox.h
+++ b/lib/kstrtox.h
@@ -2,7 +2,6 @@
 #define _LIB_KSTRTOX_H
 
 #define KSTRTOX_OVERFLOW	(1U << 31)
-const char *_parse_integer_fixup_radix(const char *s, unsigned int *base);
 unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long *res);
 
 #endif
diff --git a/lib/parse-integer.c b/lib/parse-integer.c
new file mode 100644
index 0000000..eb24307
--- /dev/null
+++ b/lib/parse-integer.c
@@ -0,0 +1,198 @@
+/*
+ * See parse_integer().
+ *
+ * Individual dispatch functions in this file aren't supposed to be used
+ * directly and thus aren't advertised and documented despited being exported.
+ *
+ * Do not use any function in this file for any reason.
+ */
+#include <linux/ctype.h>
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/math64.h>
+#include <linux/parse-integer.h>
+#include <asm/bug.h>
+
+const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
+{
+	if (*base == 0) {
+		if (s[0] == '0') {
+			if (_tolower(s[1]) == 'x' && isxdigit(s[2]))
+				*base = 16;
+			else
+				*base = 8;
+		} else
+			*base = 10;
+	}
+	if (*base == 16 && s[0] == '0' && _tolower(s[1]) == 'x')
+		s += 2;
+	BUG_ON(*base < 2 || *base > 16);
+	return s;
+}
+
+static int __parse_integer(const char *s, unsigned int base, unsigned long long *val)
+{
+	const char *s0 = s, *sd;
+	unsigned long long acc;
+
+	s = sd = _parse_integer_fixup_radix(s0, &base);
+	acc = 0;
+	while (*s) {
+		unsigned int d;
+
+		if ('0' <= *s && *s <= '9')
+			d = *s - '0';
+		else if ('a' <= _tolower(*s) && _tolower(*s) <= 'f')
+			d = _tolower(*s) - 'a' + 10;
+		else
+			break;
+		if (d >= base)
+			break;
+		/* Overflow can't happen early enough. */
+		if ((acc >> 60) && acc > div_u64(ULLONG_MAX - d, base))
+			return -ERANGE;
+		acc = acc * base + d;
+		s++;
+	}
+	/* At least one digit has to be converted. */
+	if (s == sd)
+		return -EINVAL;
+	*val = acc;
+	/* Radix 1 is not supported otherwise returned length can overflow. */
+	return s - s0;
+}
+
+int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val)
+{
+	char sign;
+	int rv;
+
+	sign = 0;
+	if (*s == '-')
+		return -EINVAL;
+	else if (*s == '+')
+		sign = *s++;
+
+	rv = __parse_integer(s, base, val);
+	if (rv < 0)
+		return rv;
+	return rv + !!sign;
+}
+EXPORT_SYMBOL(_parse_integer_ull);
+
+int _parse_integer_ll(const char *s, unsigned int base, long long *val)
+{
+	unsigned long long tmp;
+	char sign;
+	int rv;
+
+	sign = 0;
+	if (*s == '-' || *s == '+')
+		sign = *s++;
+
+	rv = __parse_integer(s, base, &tmp);
+	if (rv < 0)
+		return rv;
+	if (sign == '-') {
+		if ((long long)-tmp > 0)
+			return -ERANGE;
+		*val = -tmp;
+	} else {
+		if ((long long)tmp < 0)
+			return -ERANGE;
+		*val = tmp;
+	}
+	return rv + !!sign;
+}
+EXPORT_SYMBOL(_parse_integer_ll);
+
+int _parse_integer_u(const char *s, unsigned int base, unsigned int *val)
+{
+	unsigned long long tmp;
+	int rv;
+
+	rv = _parse_integer_ull(s, base, &tmp);
+	if (rv < 0)
+		return rv;
+	if (tmp != (unsigned int)tmp)
+		return -ERANGE;
+	*val = tmp;
+	return rv;
+}
+EXPORT_SYMBOL(_parse_integer_u);
+
+int _parse_integer_i(const char *s, unsigned int base, int *val)
+{
+	long long tmp;
+	int rv;
+
+	rv = _parse_integer_ll(s, base, &tmp);
+	if (rv < 0)
+		return rv;
+	if (tmp != (int)tmp)
+		return -ERANGE;
+	*val = tmp;
+	return rv;
+}
+EXPORT_SYMBOL(_parse_integer_i);
+
+int _parse_integer_us(const char *s, unsigned int base, unsigned short *val)
+{
+	unsigned long long tmp;
+	int rv;
+
+	rv = _parse_integer_ull(s, base, &tmp);
+	if (rv < 0)
+		return rv;
+	if (tmp != (unsigned short)tmp)
+		return -ERANGE;
+	*val = tmp;
+	return rv;
+}
+EXPORT_SYMBOL(_parse_integer_us);
+
+int _parse_integer_s(const char *s, unsigned int base, short *val)
+{
+	long long tmp;
+	int rv;
+
+	rv = _parse_integer_ll(s, base, &tmp);
+	if (rv < 0)
+		return rv;
+	if (tmp != (short)tmp)
+		return -ERANGE;
+	*val = tmp;
+	return rv;
+}
+EXPORT_SYMBOL(_parse_integer_s);
+
+int _parse_integer_uc(const char *s, unsigned int base, unsigned char *val)
+{
+	unsigned long long tmp;
+	int rv;
+
+	rv = _parse_integer_ull(s, base, &tmp);
+	if (rv < 0)
+		return rv;
+	if (tmp != (unsigned char)tmp)
+		return -ERANGE;
+	*val = tmp;
+	return rv;
+}
+EXPORT_SYMBOL(_parse_integer_uc);
+
+int _parse_integer_sc(const char *s, unsigned int base, signed char *val)
+{
+	long long tmp;
+	int rv;
+
+	rv = _parse_integer_ll(s, base, &tmp);
+	if (rv < 0)
+		return rv;
+	if (tmp != (signed char)tmp)
+		return -ERANGE;
+	*val = tmp;
+	return rv;
+}
+EXPORT_SYMBOL(_parse_integer_sc);
-- 
2.0.4

Re: [PATCH 02/12] Add parse_integer() (replacement for simple_strto*())

[Andrew Morton]

On Fri, 8 May 2015 21:30:29 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:

> kstrto*() and kstrto*_from_user() family of functions were added
> to help with parsing one integer written as string to proc/sysfs/debugfs
> files. But they have a limitation: string passed must end with \0 or \n\0.
> There are enough places where kstrto*() functions can't be used because of
> this limitation. Trivial example: major:minor "%u:%u".
> 
> Currently the only way to parse everything is simple_strto*() functions.
> But they are suboptimal:
> * they do not detect overflow (can be fixed, but no one bothered since ~0.99.11),
> * there are only 4 of them -- long and "long long" versions,
>   This leads to silent truncation in the most simple case:
> 
> 	val = strtoul(s, NULL, 0);
> 
> * half of the people think that "char **endp" argument is necessary and
>   add unnecessary variable.
> 
> OpenBSD people, fed up with how complex correct integer parsing is, added
> strtonum(3) to fixup for deficiencies of libc-style integer parsing:
> http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/strtonum.3?query=strtonum&arch=i386
> 
> It'd be OK to copy that but it relies on "errno" and fixed strings as
> error reporting channel which I think is not OK for kernel.
> strtonum() also doesn't report number of characted consumed.
> 
> What to do?
> 
> Enter parse_integer().

 fs/binfmt_misc.c               |   12 
 fs/cachefiles/daemon.c         |   84 ++--
 fs/dcache.c                    |    2 
 fs/ext2/super.c                |    6 
 fs/ext3/super.c                |    7 
 fs/ext4/super.c                |   15 
 fs/inode.c                     |    2 
 fs/libfs.c                     |   26 -
 fs/namespace.c                 |    4 
 fs/ocfs2/cluster/heartbeat.c   |   54 +-
 fs/ocfs2/cluster/nodemanager.c |   50 +-
 fs/ocfs2/stack_user.c          |   52 +-
 include/linux/kernel.h         |  129 -------
 include/linux/parse-integer.h  |  188 ++++++++++
 lib/Kconfig.debug              |    3 
 lib/Makefile                   |    2 
 lib/cmdline.c                  |   42 +-
 lib/kstrtox.c                  |  254 -------------
 lib/kstrtox.h                  |    1 
 lib/parse-integer.c            |  222 ++++++++++++
 lib/parser.c                   |   33 -
 lib/swiotlb.c                  |    2 
 lib/test-kstrtox.c             |    6 
 lib/test-parse-integer.c       |  563 +++++++++++++++++++++++++++++++
 lib/vsprintf.c                 |   81 ++--
 mm/memcontrol.c                |   19 -
 mm/memtest.c                   |    2 
 mm/page_alloc.c                |    2 
 mm/shmem.c                     |   14 
 29 files changed, 1242 insertions(+), 635 deletions(-)

So not counting lib/test-parse-integer.c, it's a net addition of 44
lines.  That's OK.

My overall reaction to this is "oh god, not again".  Is it really worth
it?


> --- /dev/null
> +++ b/include/linux/parse-integer.h
> @@ -0,0 +1,79 @@
> +#ifndef _PARSE_INTEGER_H
> +#define _PARSE_INTEGER_H
> +#include <linux/compiler.h>
> +#include <linux/types.h>
> +
> +/*
> + * int parse_integer(const char *s, unsigned int base, T *val);
> + *
> + * Convert integer string representation to an integer.
> + * Range of accepted values equals to that of type T.
> + *
> + * Conversion to unsigned integer accepts sign "+".
> + * Conversion to signed integer accepts sign "+" and sign "-".
> + *
> + * Radix 0 means autodetection: leading "0x" implies radix 16,
> + * leading "0" implies radix 8, otherwise radix is 10.
> + * Autodetection hint works after optional sign, but not before.
> + *
> + * Return number of characters parsed or -E.
> + *
> + * "T=char" case is not supported because -f{un,}signed-char can silently
> + * change range of accepted values.
> + */
> +#define parse_integer(s, base, val)	\
> +({					\
> +	const char *_s = (s);		\
> +	unsigned int _base = (base);	\
> +	typeof(&(val)[0]) _val = (val);	\
> +					\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), signed char *),	\
> +	_parse_integer_sc(_s, _base, (void *)_val),			\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), unsigned char *),	\
> +	_parse_integer_uc(_s, _base, (void *)_val),			\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), short *),		\
> +	_parse_integer_s(_s, _base, (void *)_val),			\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), unsigned short *),	\
> +	_parse_integer_us(_s, _base, (void *)_val),			\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), int *),		\
> +	_parse_integer_i(_s, _base, (void *)_val),			\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), unsigned int *),	\
> +	_parse_integer_u(_s, _base, (void *)_val),			\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), long *) && sizeof(long) == 4,\
> +	_parse_integer_i(_s, _base, (void *)_val),			\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), long *) && sizeof(long) == 8,\
> +	_parse_integer_ll(_s, _base, (void *)_val),			\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), unsigned long *) && sizeof(unsigned long) == 4,\
> +	_parse_integer_u(_s, _base, (void *)_val),			\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), unsigned long *) && sizeof(unsigned long) == 8,\
> +	_parse_integer_ull(_s, _base, (void *)_val),			\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), long long *),	\
> +	_parse_integer_ll(_s, _base, (void *)_val),			\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), unsigned long long *),\
> +	_parse_integer_ull(_s, _base, (void *)_val),			\
> +	_parse_integer_link_time_error()))))))))))));	\
> +})

Wow.

> +/* internal, do not use */
> +int _parse_integer_sc(const char *s, unsigned int base, signed char *val);
> +int _parse_integer_uc(const char *s, unsigned int base, unsigned char *val);
> +int _parse_integer_s(const char *s, unsigned int base, short *val);
> +int _parse_integer_us(const char *s, unsigned int base, unsigned short *val);
> +int _parse_integer_i(const char *s, unsigned int base, int *val);
> +int _parse_integer_u(const char *s, unsigned int base, unsigned int *val);
> +int _parse_integer_ll(const char *s, unsigned int base, long long *val);
> +int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val);

These all have fairly lengthy implementations.  Could it all be done
with a single function?

int __parse_integer(const char *s, unsigned int base, unsigned int size, void *val);

Where "size" is 1,2,4,8 with the top bit set if signed?

Re: [PATCH 02/12] Add parse_integer() (replacement for simple_strto*())

[Rasmus Villemoes]

On Fri, May 08 2015, Andrew Morton <akpm@linux-foundation.org> wrote:

> My overall reaction to this is "oh god, not again".  Is it really worth
> it?

I think it is, if it's done right. The problem is to get consensus on
what right means, but I think Alexey's approach is ok. The huge macro
may be ugly, but it puts the ugliness in one place.

>> +/* internal, do not use */
>> +int _parse_integer_sc(const char *s, unsigned int base, signed char *val);
>> +int _parse_integer_uc(const char *s, unsigned int base, unsigned char *val);
>> +int _parse_integer_s(const char *s, unsigned int base, short *val);
>> +int _parse_integer_us(const char *s, unsigned int base, unsigned short *val);
>> +int _parse_integer_i(const char *s, unsigned int base, int *val);
>> +int _parse_integer_u(const char *s, unsigned int base, unsigned int *val);
>> +int _parse_integer_ll(const char *s, unsigned int base, long long *val);
>> +int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val);
>
> These all have fairly lengthy implementations.  Could it all be done
> with a single function?
>
> int __parse_integer(const char *s, unsigned int base, unsigned int size, void *val);
>
> Where "size" is 1,2,4,8 with the top bit set if signed?

I suggested something like that in private. These two patches roughly
correspond to 02/12 and 04/12 (they are just proof-of-concept).

Subject: [PATCH 1/2] lib: introduce parse_integer

This is an alternative implementation of parse_integer. It has a
slightly smaller code footprint (both in #LOC and in .text). Another
motivation was to expand on the idea of passing flags to the
underlying function, easing implementation of other interfaces in
terms of parse_integer.

In the other proposal, PARSE_INTEGER_NEWLINE means three things, which
I split into separate flags:

* Accept (but don't require) a single trailing newline.

* Require that the entire string is consumed (possibly after eating
  the trailing newline), otherwise return -EINVAL.

* Change return semantics: 0 for ok instead of #characters consumed.

Besides the three flags doing the above, I also added flags allowing
consuming leading and/or trailing whitespace. This may be used to
remove some boilerplate from code elsewhere. But this may be
over-engineering at this point, and it's easy enough to rip out (and
maybe add later). Still, I think it's nice to keep the
behaviour-changing flags separate.

Implementation-wise (and what saves ~500 bytes of .text), the main
difference is that there's only a single underlying function, and the
type of the destination is communicated to it with another few bits in
the base parameter. In the vast majority of cases, the combined
base_type_flags parameter will be a compile-time constant.

---
 include/linux/parse-integer.h |  78 ++++++++++++++++++++
 lib/Makefile                  |   1 +
 lib/kstrtox.c                 |  16 ----
 lib/kstrtox.h                 |   3 +-
 lib/parse-integer.c           | 166 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 247 insertions(+), 17 deletions(-)
 create mode 100644 include/linux/parse-integer.h
 create mode 100644 lib/parse-integer.c

diff --git a/include/linux/parse-integer.h b/include/linux/parse-integer.h
new file mode 100644
index 000000000000..bdeb6c06059a
--- /dev/null
+++ b/include/linux/parse-integer.h
@@ -0,0 +1,78 @@
+#ifndef LINUX_PARSE_INTEGER_H
+#define LINUX_PARSE_INTEGER_H
+
+#include <linux/compiler.h>
+#include <linux/types.h>
+
+#define _PARSE_INTEGER_TYPE_SHIFT 8
+enum {
+	_PARSE_INTEGER_U8,
+	_PARSE_INTEGER_U16,
+	_PARSE_INTEGER_U32,
+	_PARSE_INTEGER_U64,
+	_PARSE_INTEGER_S8,
+	_PARSE_INTEGER_S16,
+	_PARSE_INTEGER_S32,
+	_PARSE_INTEGER_S64,
+};
+
+/*
+ * Various flags that can be ORed with the base to request slightly
+ * different semantics, to facilitate implementing various old
+ * interfaces in terms of parse_integer. Not to be used directly.
+ */
+
+/* allow (and skip) leading whitespace */
+#define _PARSE_INTEGER_LEAD_WS    0x08000000
+/* consume trailing whitespace */
+#define _PARSE_INTEGER_TRAIL_WS   0x10000000
+/* consume a single trailing newline */
+#define _PARSE_INTEGER_NEWLINE    0x20000000
+/* return 0 for success instead of #characters consumed */
+#define _PARSE_INTEGER_ZERO_ON_OK 0x40000000
+/* return -EINVAL unless the entire string was consumed */
+#define _PARSE_INTEGER_ALL        0x80000000
+
+/*
+ * This should just be BUILD_BUG(), but due to header dependency hell
+ * we can't use that.
+ */
+void __parse_integer_build_bug(void);
+
+#define parse_integer(s, base_flags, dest) ({			\
+	const char *_s = (s);					\
+	unsigned _btf = (base_flags);				\
+	typeof(&dest[0]) _dest = (dest);			\
+	unsigned _t;						\
+								\
+	if (__builtin_types_compatible_p(typeof(_dest), u8*))		\
+		_t = _PARSE_INTEGER_U8;					\
+	else if (__builtin_types_compatible_p(typeof(_dest), u16*))	\
+		_t = _PARSE_INTEGER_U16;				\
+	else if (__builtin_types_compatible_p(typeof(_dest), u32*))	\
+		_t = _PARSE_INTEGER_U32;				\
+	else if (__builtin_types_compatible_p(typeof(_dest), u64*))	\
+		_t = _PARSE_INTEGER_U64;				\
+	else if (__builtin_types_compatible_p(typeof(_dest), s8*))	\
+		_t = _PARSE_INTEGER_S8;					\
+	else if (__builtin_types_compatible_p(typeof(_dest), s16*))	\
+		_t = _PARSE_INTEGER_S16;				\
+	else if (__builtin_types_compatible_p(typeof(_dest), s32*))	\
+		_t = _PARSE_INTEGER_S32;				\
+	else if (__builtin_types_compatible_p(typeof(_dest), s64*))	\
+		_t = _PARSE_INTEGER_S64;				\
+	else if (__builtin_types_compatible_p(typeof(_dest), unsigned long*)) \
+		_t = BITS_PER_LONG == 64 ? _PARSE_INTEGER_U64 : _PARSE_INTEGER_U32; \
+	else if (__builtin_types_compatible_p(typeof(_dest), long*))	\
+		_t = BITS_PER_LONG == 64 ? _PARSE_INTEGER_S64 : _PARSE_INTEGER_S32; \
+	else								\
+		__parse_integer_build_bug();				\
+	_btf |= _t << _PARSE_INTEGER_TYPE_SHIFT;			\
+	__parse_integer(_s, _btf, _dest);				\
+})
+
+/* internal, do not use directly */
+int __parse_integer(const char *s, unsigned base_type_flags, void *val);
+const char *_parse_integer_fixup_radix(const char *s, unsigned int *base);
+
+#endif /* LINUX_PARSE_INTEGER_H */
diff --git a/lib/Makefile b/lib/Makefile
index 6c37933336a0..e69b356fe327 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
 obj-y += hexdump.o
 obj-$(CONFIG_TEST_HEXDUMP) += test-hexdump.o
 obj-y += kstrtox.o
+obj-y += parse-integer.o
 obj-$(CONFIG_TEST_BPF) += test_bpf.o
 obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o
 obj-$(CONFIG_TEST_KASAN) += test_kasan.o
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index ec8da78df9be..5b26f283428f 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -20,22 +20,6 @@
 #include <asm/uaccess.h>
 #include "kstrtox.h"
 
-const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
-{
-	if (*base == 0) {
-		if (s[0] == '0') {
-			if (_tolower(s[1]) == 'x' && isxdigit(s[2]))
-				*base = 16;
-			else
-				*base = 8;
-		} else
-			*base = 10;
-	}
-	if (*base == 16 && s[0] == '0' && _tolower(s[1]) == 'x')
-		s += 2;
-	return s;
-}
-
 /*
  * Convert non-negative integer string representation in explicitly given radix
  * to an integer.
diff --git a/lib/kstrtox.h b/lib/kstrtox.h
index f13eeeaf441d..44d9ce227164 100644
--- a/lib/kstrtox.h
+++ b/lib/kstrtox.h
@@ -1,8 +1,9 @@
 #ifndef _LIB_KSTRTOX_H
 #define _LIB_KSTRTOX_H
 
+#include <linux/parse-integer.h>
+
 #define KSTRTOX_OVERFLOW	(1U << 31)
-const char *_parse_integer_fixup_radix(const char *s, unsigned int *base);
 unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long *res);
 
 #endif
diff --git a/lib/parse-integer.c b/lib/parse-integer.c
new file mode 100644
index 000000000000..88f60a02c019
--- /dev/null
+++ b/lib/parse-integer.c
@@ -0,0 +1,166 @@
+#include <linux/bug.h>
+#include <linux/ctype.h>
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/math64.h>
+#include <linux/parse-integer.h>
+#include <linux/string.h>
+
+const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
+{
+	if (*base == 0) {
+		if (s[0] == '0') {
+			if (_tolower(s[1]) == 'x' && isxdigit(s[2]))
+				*base = 16;
+			else
+				*base = 8;
+		} else
+			*base = 10;
+	}
+	if (*base == 16 && s[0] == '0' && _tolower(s[1]) == 'x')
+		s += 2;
+	BUG_ON(*base < 2 || *base > 16);
+	return s;
+}
+
+
+static int do_parse_integer(const char *buf, unsigned base, u64 *dest)
+{
+	const char *s = buf, *s0;
+	u64 acc = 0;
+
+	s = _parse_integer_fixup_radix(s, &base);
+	s0 = s;
+	while (1) {
+		unsigned int d;
+
+		if ('0' <= *s && *s <= '9')
+			d = *s - '0';
+		else if ('a' <= _tolower(*s) && _tolower(*s) <= 'f')
+			d = _tolower(*s) - 'a' + 10;
+		else
+			break;
+		if (d >= base)
+			break;
+		/* Overflow can't happen early enough. */
+		if ((acc >> 60) && acc > div_u64(ULLONG_MAX - d, base))
+			return -ERANGE;
+		acc = acc * base + d;
+		s++;
+	}
+	/* At least one digit has to be converted. */
+	if (s == s0)
+		return -EINVAL;
+	*dest = acc;
+	return s - buf;
+}
+
+int __parse_integer(const char *buf, unsigned btf, void *dest)
+{
+	unsigned base = btf & 0xff;
+	unsigned type = (btf >> _PARSE_INTEGER_TYPE_SHIFT) & 7;
+	unsigned flags = btf & 0xff000000;
+	const char *s = buf;
+	union {
+		s64 s;
+		u64 u;
+	} val;
+	int neg = 0;
+	int rv;
+
+	if (flags & _PARSE_INTEGER_LEAD_WS)
+		s = skip_spaces(s);
+	
+	if (*s == '-') {
+		/* We don't even allow "-0" for unsigned types. */
+		if (type <= _PARSE_INTEGER_U64)
+			return -EINVAL;
+		neg = 1;
+		s++;
+	} else if (*s == '+') {
+		s++;
+	}
+
+	rv = do_parse_integer(s, base, &val.u);
+	if (rv < 0)
+		return rv;
+	s += rv;
+
+	/*
+	 * If we're targetting an unsigned type, proper range checking
+	 * for val.u has already been done. If we're targeting a
+	 * signed type, we may need to apply the minus sign and then
+	 * check that the result has the expected sign. We do allow
+	 * both -0 and +0.
+	 */
+	if (type >= _PARSE_INTEGER_S8) {
+		if (neg)
+			val.s = -val.u;
+		else
+			val.s = val.u;
+		if ((neg && val.s > 0) || (!neg && val.s < 0))
+			return -ERANGE;
+	}
+
+	if (flags & _PARSE_INTEGER_NEWLINE && *s == '\n')
+		++s;
+	else if (flags & _PARSE_INTEGER_TRAIL_WS)
+		s = skip_spaces(s);
+
+	if (flags & _PARSE_INTEGER_ALL && *s++ != '\0')
+		return -EINVAL;
+
+	/*
+	 * So far so good. Do final range check and write to the
+	 * destination if ok.
+	 */
+	switch (type) {
+	case _PARSE_INTEGER_U8:
+		if ((u8)val.u != val.u)
+			return -ERANGE;
+		*(u8*)dest = val.u;
+		break;
+	case _PARSE_INTEGER_U16:
+		if ((u16)val.u != val.u)
+			return -ERANGE;
+		*(u16*)dest = val.u;
+		break;
+	case _PARSE_INTEGER_U32:
+		if ((u32)val.u != val.u)
+			return -ERANGE;
+		*(u32*)dest = val.u;
+		break;
+	case _PARSE_INTEGER_U64:
+		/* if ((u64)val.u != val.u) */
+		/* 	return -ERANGE; */
+		*(u64*)dest = val.u;
+		break;
+	case _PARSE_INTEGER_S8:
+		if ((s8)val.s != val.s)
+			return -ERANGE;
+		*(s8*)dest = val.s;
+		break;
+	case _PARSE_INTEGER_S16:
+		if ((s16)val.s != val.s)
+			return -ERANGE;
+		*(s16*)dest = val.s;
+		break;
+	case _PARSE_INTEGER_S32:
+		if ((s32)val.s != val.s)
+			return -ERANGE;
+		*(s32*)dest = val.s;
+		break;
+	case _PARSE_INTEGER_S64:
+		/* if ((s64)val.s != val.s) */
+		/* 	return -ERANGE; */
+		*(s64*)dest = val.s;
+		break;
+	default:
+		BUG();
+	}
+
+	if (flags & _PARSE_INTEGER_ZERO_ON_OK)
+		return 0;
+	return s - buf;
+}
+EXPORT_SYMBOL(__parse_integer);
-- 
2.1.3


Subject: [PATCH 2/2] lib: implement kstrtox using parse_integer

The new parse_integer() interface makes it easy to implement the
kstrtox family of functions as simple one-line static inlines.
---
 include/linux/kernel.h        | 125 +---------------------
 include/linux/parse-integer.h | 112 ++++++++++++++++++++
 lib/kstrtox.c                 | 238 ------------------------------------------
 3 files changed, 113 insertions(+), 362 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3a5b48e52a9e..27465774e00a 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -14,6 +14,7 @@
 #include <linux/dynamic_debug.h>
 #include <asm/byteorder.h>
 #include <uapi/linux/kernel.h>
+#include <linux/parse-integer.h>
 
 #define USHRT_MAX	((u16)(~0U))
 #define SHRT_MAX	((s16)(USHRT_MAX>>1))
@@ -263,130 +264,6 @@ void do_exit(long error_code)
 void complete_and_exit(struct completion *, long)
 	__noreturn;
 
-/* Internal, do not use. */
-int __must_check _kstrtoul(const char *s, unsigned int base, unsigned long *res);
-int __must_check _kstrtol(const char *s, unsigned int base, long *res);
-
-int __must_check kstrtoull(const char *s, unsigned int base, unsigned long long *res);
-int __must_check kstrtoll(const char *s, unsigned int base, long long *res);
-
-/**
- * kstrtoul - convert a string to an unsigned long
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign, but not a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
-*/
-static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
-{
-	/*
-	 * We want to shortcut function call, but
-	 * __builtin_types_compatible_p(unsigned long, unsigned long long) = 0.
-	 */
-	if (sizeof(unsigned long) == sizeof(unsigned long long) &&
-	    __alignof__(unsigned long) == __alignof__(unsigned long long))
-		return kstrtoull(s, base, (unsigned long long *)res);
-	else
-		return _kstrtoul(s, base, res);
-}
-
-/**
- * kstrtol - convert a string to a long
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign or a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
- */
-static inline int __must_check kstrtol(const char *s, unsigned int base, long *res)
-{
-	/*
-	 * We want to shortcut function call, but
-	 * __builtin_types_compatible_p(long, long long) = 0.
-	 */
-	if (sizeof(long) == sizeof(long long) &&
-	    __alignof__(long) == __alignof__(long long))
-		return kstrtoll(s, base, (long long *)res);
-	else
-		return _kstrtol(s, base, res);
-}
-
-int __must_check kstrtouint(const char *s, unsigned int base, unsigned int *res);
-int __must_check kstrtoint(const char *s, unsigned int base, int *res);
-
-static inline int __must_check kstrtou64(const char *s, unsigned int base, u64 *res)
-{
-	return kstrtoull(s, base, res);
-}
-
-static inline int __must_check kstrtos64(const char *s, unsigned int base, s64 *res)
-{
-	return kstrtoll(s, base, res);
-}
-
-static inline int __must_check kstrtou32(const char *s, unsigned int base, u32 *res)
-{
-	return kstrtouint(s, base, res);
-}
-
-static inline int __must_check kstrtos32(const char *s, unsigned int base, s32 *res)
-{
-	return kstrtoint(s, base, res);
-}
-
-int __must_check kstrtou16(const char *s, unsigned int base, u16 *res);
-int __must_check kstrtos16(const char *s, unsigned int base, s16 *res);
-int __must_check kstrtou8(const char *s, unsigned int base, u8 *res);
-int __must_check kstrtos8(const char *s, unsigned int base, s8 *res);
-
-int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
-int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
-int __must_check kstrtoul_from_user(const char __user *s, size_t count, unsigned int base, unsigned long *res);
-int __must_check kstrtol_from_user(const char __user *s, size_t count, unsigned int base, long *res);
-int __must_check kstrtouint_from_user(const char __user *s, size_t count, unsigned int base, unsigned int *res);
-int __must_check kstrtoint_from_user(const char __user *s, size_t count, unsigned int base, int *res);
-int __must_check kstrtou16_from_user(const char __user *s, size_t count, unsigned int base, u16 *res);
-int __must_check kstrtos16_from_user(const char __user *s, size_t count, unsigned int base, s16 *res);
-int __must_check kstrtou8_from_user(const char __user *s, size_t count, unsigned int base, u8 *res);
-int __must_check kstrtos8_from_user(const char __user *s, size_t count, unsigned int base, s8 *res);
-
-static inline int __must_check kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)
-{
-	return kstrtoull_from_user(s, count, base, res);
-}
-
-static inline int __must_check kstrtos64_from_user(const char __user *s, size_t count, unsigned int base, s64 *res)
-{
-	return kstrtoll_from_user(s, count, base, res);
-}
-
-static inline int __must_check kstrtou32_from_user(const char __user *s, size_t count, unsigned int base, u32 *res)
-{
-	return kstrtouint_from_user(s, count, base, res);
-}
-
-static inline int __must_check kstrtos32_from_user(const char __user *s, size_t count, unsigned int base, s32 *res)
-{
-	return kstrtoint_from_user(s, count, base, res);
-}
-
 /* Obsolete, do not use.  Use kstrto<foo> instead */
 
 extern unsigned long simple_strtoul(const char *,char **,unsigned int);
diff --git a/include/linux/parse-integer.h b/include/linux/parse-integer.h
index bdeb6c06059a..3deebaa3e547 100644
--- a/include/linux/parse-integer.h
+++ b/include/linux/parse-integer.h
@@ -33,6 +33,10 @@ enum {
 /* return -EINVAL unless the entire string was consumed */
 #define _PARSE_INTEGER_ALL        0x80000000
 
+/* The kstrtox family can be implemented using this combination. */
+#define _PARSE_INTEGER_KSTRTOX \
+	(_PARSE_INTEGER_NEWLINE | _PARSE_INTEGER_ZERO_ON_OK | _PARSE_INTEGER_ALL)
+
 /*
  * This should just be BUILD_BUG(), but due to header dependency hell
  * we can't use that.
@@ -75,4 +79,112 @@ void __parse_integer_build_bug(void);
 int __parse_integer(const char *s, unsigned base_type_flags, void *val);
 const char *_parse_integer_fixup_radix(const char *s, unsigned int *base);
 
+/*
+ * Convert integer string representation terminated by \n\0 or \0 to an integer.
+ *
+ * Return 0 on success or -E.
+ *
+ * See parse_integer().
+ */
+static inline int __must_check kstrtoull(const char *s, unsigned int base, unsigned long long *res)
+{
+	return parse_integer(s, base | _PARSE_INTEGER_KSTRTOX, res);
+}
+
+static inline int __must_check kstrtoll(const char *s, unsigned int base, long long *res)
+{
+	return parse_integer(s, base | _PARSE_INTEGER_KSTRTOX, res);
+}
+
+static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
+{
+	return parse_integer(s, base | _PARSE_INTEGER_KSTRTOX, res);
+}
+
+static inline int __must_check kstrtol(const char *s, unsigned int base, long *res)
+{
+	return parse_integer(s, base | _PARSE_INTEGER_KSTRTOX, res);
+}
+
+static inline int __must_check kstrtouint(const char *s, unsigned int base, unsigned int *res)
+{
+	return parse_integer(s, base | _PARSE_INTEGER_KSTRTOX, res);
+}
+
+static inline int __must_check kstrtoint(const char *s, unsigned int base, int *res)
+{
+	return parse_integer(s, base | _PARSE_INTEGER_KSTRTOX, res);
+}
+
+static inline int __must_check kstrtou64(const char *s, unsigned int base, u64 *res)
+{
+	return kstrtoull(s, base, res);
+}
+
+static inline int __must_check kstrtos64(const char *s, unsigned int base, s64 *res)
+{
+	return kstrtoll(s, base, res);
+}
+
+static inline int __must_check kstrtou32(const char *s, unsigned int base, u32 *res)
+{
+	return kstrtouint(s, base, res);
+}
+
+static inline int __must_check kstrtos32(const char *s, unsigned int base, s32 *res)
+{
+	return kstrtoint(s, base, res);
+}
+
+static inline int __must_check kstrtou16(const char *s, unsigned int base, u16 *res)
+{
+	return parse_integer(s, base | _PARSE_INTEGER_KSTRTOX, res);
+}
+
+static inline int __must_check kstrtos16(const char *s, unsigned int base, s16 *res)
+{
+	return parse_integer(s, base | _PARSE_INTEGER_KSTRTOX, res);
+}
+
+static inline int __must_check kstrtou8(const char *s, unsigned int base, u8 *res)
+{
+	return parse_integer(s, base | _PARSE_INTEGER_KSTRTOX, res);
+}
+
+static inline int __must_check kstrtos8(const char *s, unsigned int base, s8 *res)
+{
+	return parse_integer(s, base | _PARSE_INTEGER_KSTRTOX, res);
+}
+
+int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
+int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
+int __must_check kstrtoul_from_user(const char __user *s, size_t count, unsigned int base, unsigned long *res);
+int __must_check kstrtol_from_user(const char __user *s, size_t count, unsigned int base, long *res);
+int __must_check kstrtouint_from_user(const char __user *s, size_t count, unsigned int base, unsigned int *res);
+int __must_check kstrtoint_from_user(const char __user *s, size_t count, unsigned int base, int *res);
+int __must_check kstrtou16_from_user(const char __user *s, size_t count, unsigned int base, u16 *res);
+int __must_check kstrtos16_from_user(const char __user *s, size_t count, unsigned int base, s16 *res);
+int __must_check kstrtou8_from_user(const char __user *s, size_t count, unsigned int base, u8 *res);
+int __must_check kstrtos8_from_user(const char __user *s, size_t count, unsigned int base, s8 *res);
+
+static inline int __must_check kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)
+{
+	return kstrtoull_from_user(s, count, base, res);
+}
+
+static inline int __must_check kstrtos64_from_user(const char __user *s, size_t count, unsigned int base, s64 *res)
+{
+	return kstrtoll_from_user(s, count, base, res);
+}
+
+static inline int __must_check kstrtou32_from_user(const char __user *s, size_t count, unsigned int base, u32 *res)
+{
+	return kstrtouint_from_user(s, count, base, res);
+}
+
+static inline int __must_check kstrtos32_from_user(const char __user *s, size_t count, unsigned int base, s32 *res)
+{
+	return kstrtoint_from_user(s, count, base, res);
+}
+
 #endif /* LINUX_PARSE_INTEGER_H */
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index 5b26f283428f..1698b286d954 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -67,244 +67,6 @@ unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long
 	return rv;
 }
 
-static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
-{
-	unsigned long long _res;
-	unsigned int rv;
-
-	s = _parse_integer_fixup_radix(s, &base);
-	rv = _parse_integer(s, base, &_res);
-	if (rv & KSTRTOX_OVERFLOW)
-		return -ERANGE;
-	if (rv == 0)
-		return -EINVAL;
-	s += rv;
-	if (*s == '\n')
-		s++;
-	if (*s)
-		return -EINVAL;
-	*res = _res;
-	return 0;
-}
-
-/**
- * kstrtoull - convert a string to an unsigned long long
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign, but not a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
- */
-int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
-{
-	if (s[0] == '+')
-		s++;
-	return _kstrtoull(s, base, res);
-}
-EXPORT_SYMBOL(kstrtoull);
-
-/**
- * kstrtoll - convert a string to a long long
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign or a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
- */
-int kstrtoll(const char *s, unsigned int base, long long *res)
-{
-	unsigned long long tmp;
-	int rv;
-
-	if (s[0] == '-') {
-		rv = _kstrtoull(s + 1, base, &tmp);
-		if (rv < 0)
-			return rv;
-		if ((long long)(-tmp) >= 0)
-			return -ERANGE;
-		*res = -tmp;
-	} else {
-		rv = kstrtoull(s, base, &tmp);
-		if (rv < 0)
-			return rv;
-		if ((long long)tmp < 0)
-			return -ERANGE;
-		*res = tmp;
-	}
-	return 0;
-}
-EXPORT_SYMBOL(kstrtoll);
-
-/* Internal, do not use. */
-int _kstrtoul(const char *s, unsigned int base, unsigned long *res)
-{
-	unsigned long long tmp;
-	int rv;
-
-	rv = kstrtoull(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (unsigned long long)(unsigned long)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(_kstrtoul);
-
-/* Internal, do not use. */
-int _kstrtol(const char *s, unsigned int base, long *res)
-{
-	long long tmp;
-	int rv;
-
-	rv = kstrtoll(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (long long)(long)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(_kstrtol);
-
-/**
- * kstrtouint - convert a string to an unsigned int
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign, but not a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
- */
-int kstrtouint(const char *s, unsigned int base, unsigned int *res)
-{
-	unsigned long long tmp;
-	int rv;
-
-	rv = kstrtoull(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (unsigned long long)(unsigned int)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtouint);
-
-/**
- * kstrtoint - convert a string to an int
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign or a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
- */
-int kstrtoint(const char *s, unsigned int base, int *res)
-{
-	long long tmp;
-	int rv;
-
-	rv = kstrtoll(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (long long)(int)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtoint);
-
-int kstrtou16(const char *s, unsigned int base, u16 *res)
-{
-	unsigned long long tmp;
-	int rv;
-
-	rv = kstrtoull(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (unsigned long long)(u16)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtou16);
-
-int kstrtos16(const char *s, unsigned int base, s16 *res)
-{
-	long long tmp;
-	int rv;
-
-	rv = kstrtoll(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (long long)(s16)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtos16);
-
-int kstrtou8(const char *s, unsigned int base, u8 *res)
-{
-	unsigned long long tmp;
-	int rv;
-
-	rv = kstrtoull(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (unsigned long long)(u8)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtou8);
-
-int kstrtos8(const char *s, unsigned int base, s8 *res)
-{
-	long long tmp;
-	int rv;
-
-	rv = kstrtoll(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (long long)(s8)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtos8);
-
 #define kstrto_from_user(f, g, type)					\
 int f(const char __user *s, size_t count, unsigned int base, type *res)	\
 {									\
-- 
2.1.3

Re: [PATCH 02/12] Add parse_integer() (replacement for simple_strto*())

[Alexey Dobriyan]

On Fri, May 08, 2015 at 01:46:46PM -0700, Andrew Morton wrote:
> On Fri, 8 May 2015 21:30:29 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:
> 
> > kstrto*() and kstrto*_from_user() family of functions were added
> > to help with parsing one integer written as string to proc/sysfs/debugfs
> > files. But they have a limitation: string passed must end with \0 or \n\0.
> > There are enough places where kstrto*() functions can't be used because of
> > this limitation. Trivial example: major:minor "%u:%u".
> > 
> > Currently the only way to parse everything is simple_strto*() functions.
> > But they are suboptimal:
> > * they do not detect overflow (can be fixed, but no one bothered since ~0.99.11),
> > * there are only 4 of them -- long and "long long" versions,
> >   This leads to silent truncation in the most simple case:
> > 
> > 	val = strtoul(s, NULL, 0);
> > 
> > * half of the people think that "char **endp" argument is necessary and
> >   add unnecessary variable.
> > 
> > OpenBSD people, fed up with how complex correct integer parsing is, added
> > strtonum(3) to fixup for deficiencies of libc-style integer parsing:
> > http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/strtonum.3?query=strtonum&arch=i386
> > 
> > It'd be OK to copy that but it relies on "errno" and fixed strings as
> > error reporting channel which I think is not OK for kernel.
> > strtonum() also doesn't report number of characted consumed.
> > 
> > What to do?
> > 
> > Enter parse_integer().
> 
>  fs/binfmt_misc.c               |   12 
>  fs/cachefiles/daemon.c         |   84 ++--
>  fs/dcache.c                    |    2 
>  fs/ext2/super.c                |    6 
>  fs/ext3/super.c                |    7 
>  fs/ext4/super.c                |   15 
>  fs/inode.c                     |    2 
>  fs/libfs.c                     |   26 -
>  fs/namespace.c                 |    4 
>  fs/ocfs2/cluster/heartbeat.c   |   54 +-
>  fs/ocfs2/cluster/nodemanager.c |   50 +-
>  fs/ocfs2/stack_user.c          |   52 +-
>  include/linux/kernel.h         |  129 -------
>  include/linux/parse-integer.h  |  188 ++++++++++
>  lib/Kconfig.debug              |    3 
>  lib/Makefile                   |    2 
>  lib/cmdline.c                  |   42 +-
>  lib/kstrtox.c                  |  254 -------------
>  lib/kstrtox.h                  |    1 
>  lib/parse-integer.c            |  222 ++++++++++++
>  lib/parser.c                   |   33 -
>  lib/swiotlb.c                  |    2 
>  lib/test-kstrtox.c             |    6 
>  lib/test-parse-integer.c       |  563 +++++++++++++++++++++++++++++++
>  lib/vsprintf.c                 |   81 ++--
>  mm/memcontrol.c                |   19 -
>  mm/memtest.c                   |    2 
>  mm/page_alloc.c                |    2 
>  mm/shmem.c                     |   14 
>  29 files changed, 1242 insertions(+), 635 deletions(-)
> 
> So not counting lib/test-parse-integer.c, it's a net addition of 44
> lines.  That's OK.
> 
> My overall reaction to this is "oh god, not again".  Is it really worth
> it?

I think giving good examples to people is always worth it :-)

> > +#define parse_integer(s, base, val)	\
> > +({					\
> > +	const char *_s = (s);		\
> > +	unsigned int _base = (base);	\
> > +	typeof(&(val)[0]) _val = (val);	\
> > +					\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), signed char *),	\
> > +	_parse_integer_sc(_s, _base, (void *)_val),			\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), unsigned char *),	\
> > +	_parse_integer_uc(_s, _base, (void *)_val),			\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), short *),		\
> > +	_parse_integer_s(_s, _base, (void *)_val),			\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), unsigned short *),	\
> > +	_parse_integer_us(_s, _base, (void *)_val),			\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), int *),		\
> > +	_parse_integer_i(_s, _base, (void *)_val),			\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), unsigned int *),	\
> > +	_parse_integer_u(_s, _base, (void *)_val),			\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), long *) && sizeof(long) == 4,\
> > +	_parse_integer_i(_s, _base, (void *)_val),			\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), long *) && sizeof(long) == 8,\
> > +	_parse_integer_ll(_s, _base, (void *)_val),			\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), unsigned long *) && sizeof(unsigned long) == 4,\
> > +	_parse_integer_u(_s, _base, (void *)_val),			\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), unsigned long *) && sizeof(unsigned long) == 8,\
> > +	_parse_integer_ull(_s, _base, (void *)_val),			\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), long long *),	\
> > +	_parse_integer_ll(_s, _base, (void *)_val),			\
> > +	__builtin_choose_expr(						\
> > +	__builtin_types_compatible_p(typeof(_val), unsigned long long *),\
> > +	_parse_integer_ull(_s, _base, (void *)_val),			\
> > +	_parse_integer_link_time_error()))))))))))));	\
> > +})
> 
> Wow.

Yep, but blame the language not me.
Macro would be simpler with _Generic, but it is too early for that.

> > +/* internal, do not use */
> > +int _parse_integer_sc(const char *s, unsigned int base, signed char *val);
> > +int _parse_integer_uc(const char *s, unsigned int base, unsigned char *val);
> > +int _parse_integer_s(const char *s, unsigned int base, short *val);
> > +int _parse_integer_us(const char *s, unsigned int base, unsigned short *val);
> > +int _parse_integer_i(const char *s, unsigned int base, int *val);
> > +int _parse_integer_u(const char *s, unsigned int base, unsigned int *val);
> > +int _parse_integer_ll(const char *s, unsigned int base, long long *val);
> > +int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val);
> 
> These all have fairly lengthy implementations.  Could it all be done
> with a single function?
> 
> int __parse_integer(const char *s, unsigned int base, unsigned int size, void *val);
> 
> Where "size" is 1,2,4,8 with the top bit set if signed?

The question is why bother.

With smallish VM config I was testing, .text size difference is very small,
LOC-wise the difference doesn't exist as well (additional error checking adds
lines, not code per se). In the end _parse_integer() and simple_strto*() functions
will be removed as well.

With 4-arg dispatch function, every callsite will be more bloated:

	simple_strto*	(pointer, pointer, int)
	parse_integer	(pointer, int, pointer)
	4-arg		(pointer, int, int, pointer)

I think this code is way understandable than any mask-shift alternative:

	int _parse_integer_sc(const char *s, unsigned int base, signed char *val)
	{
	        long long tmp;
	        int rv;

	        rv = _parse_integer_ll(s, base, &tmp);
	        if (rv < 0)
	                return rv;
	        if (tmp != (signed char)tmp)
	                return -ERANGE;
	        *val = tmp;
	        return rv;
	}

Re: [PATCH 02/12] Add parse_integer() (replacement for simple_strto*())

[Alexey Dobriyan]

[Answering to Noel Grandin, gmane sent email to list only]

> > int parse_integer(const char *s, unsigned int base, T *val);
> >
>
> I suspect splitting this into
>      parse_signed_int
> and
>     parse_unsigned_int
> (or some other equivalent names)
> will make the call-sites a lot easier to read.

parse_integer() is about two and only two points (everything else is
immaterial and technical details):
1) unified interface,
2) automatically fixing overflow bug based on type T.

Two interfaces in this situation is no different from having four.

C doesn't have generic functions so programmers get hardwired
to million of names all doing roughly the same but __b_c_e and
_Generic allows to mimic generic functions which is at least something.

Re: [PATCH 02/12] Add parse_integer() (replacement for simple_strto*())

[Noel Grandin]

> 	int parse_integer(const char *s, unsigned int base, T *val);
> 

I suspect splitting this into 
     parse_signed_int
and
    parse_unsigned_int
(or some other equivalent names)
will make the call-sites a lot easier to read.

Re: [PATCH 02/12] Add parse_integer() (replacement for simple_strto*())

[Andrew Morton]

On Fri, 8 May 2015 21:30:29 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:

> Enter parse_integer().
> 
> 	int parse_integer(const char *s, unsigned int base, T *val);

OK, I grabbed these.  Let's see how it goes.  Were these patches still
up to date?

Presumably we'd benefit from a checkpatch rule to alert people to the
new regime.  Can you please suggest what such a rule should do?

The macros in lib/test-parse-integer.c hurt my brain.

Re: [PATCH 02/12] Add parse_integer() (replacement for simple_strto*())

[Alexey Dobriyan]

On Thu, Jul 09, 2015 at 12:28:41PM -0700, Andrew Morton wrote:
> On Fri, 8 May 2015 21:30:29 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:
> 
> > Enter parse_integer().
> > 
> > 	int parse_integer(const char *s, unsigned int base, T *val);
> 
> OK, I grabbed these.

Thanks! Just to note, first patch (accept "-0") is independent and
can be sent at any time.

> Let's see how it goes.  Were these patches still up to date?

Yes, kstrto*(), scanf() didn't change since when this was sent.

> Presumably we'd benefit from a checkpatch rule to alert people to the
> new regime.  Can you please suggest what such a rule should do?

It should say to switch from simple_strto*() to one of the 4 officially
sanctioned integer conversion routines: parse_integer(), kstrto*(),
kstrto*_from_user(), sscanf(). I'll send a patch.

> The macros in lib/test-parse-integer.c hurt my brain.

Well, yes, but the only interesting thing there are test tables.

	Alexey

[PATCH 03/12] parse_integer: add runtime testsuite

[Alexey Dobriyan]

Use with CONFIG_PARSE_INTEGER=y.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 lib/Kconfig.debug        |   3 +
 lib/Makefile             |   1 +
 lib/test-parse-integer.c | 563 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 567 insertions(+)
 create mode 100644 lib/test-parse-integer.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ba2b0c8..35224ca 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1628,6 +1628,9 @@ config TEST_HEXDUMP
 config TEST_STRING_HELPERS
 	tristate "Test functions located in the string_helpers module at runtime"
 
+config TEST_PARSE_INTEGER
+	tristate "Test parse_integer() function at runtime"
+
 config TEST_KSTRTOX
 	tristate "Test kstrto*() family of functions at runtime"
 
diff --git a/lib/Makefile b/lib/Makefile
index e69b356..1925810 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -36,6 +36,7 @@ obj-y += parse-integer.o
 obj-$(CONFIG_TEST_BPF) += test_bpf.o
 obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o
 obj-$(CONFIG_TEST_KASAN) += test_kasan.o
+obj-$(CONFIG_TEST_PARSE_INTEGER) += test-parse-integer.o
 obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
 obj-$(CONFIG_TEST_LKM) += test_module.o
 obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.o
diff --git a/lib/test-parse-integer.c b/lib/test-parse-integer.c
new file mode 100644
index 0000000..4274603
--- /dev/null
+++ b/lib/test-parse-integer.c
@@ -0,0 +1,563 @@
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/parse-integer.h>
+#include <asm/bug.h>
+
+#define for_each_test(i, test)  \
+	for (i = 0; i < ARRAY_SIZE(test); i++)
+
+#define DEFINE_TEST_OK(type, test_type, test)	\
+test_type {					\
+	const char *str;			\
+	unsigned int base;			\
+	int expected_rv;			\
+	type expected_val;			\
+};						\
+static const test_type test[] __initconst =
+
+#define TEST_OK(type, fmt, test)				\
+{								\
+	unsigned int i;						\
+								\
+	for_each_test(i, test) {				\
+		const typeof(test[0]) *t = &test[i];		\
+		type val;					\
+		int rv;						\
+								\
+		rv = parse_integer(t->str, t->base, &val);	\
+		if (rv != t->expected_rv || val != t->expected_val) {				\
+			WARN(1, "str '%s', base %u, expected %d/"fmt", got %d/"fmt"\n",		\
+				t->str, t->base, t->expected_rv, t->expected_val, rv, val);	\
+		}						\
+	}							\
+}
+
+struct test_fail {
+	const char *str;
+	unsigned int base;
+};
+
+#define DEFINE_TEST_FAIL(type, test)	\
+static const struct test_fail test[] __initconst =
+
+#define TEST_FAIL(type, fmt, test)				\
+{								\
+	unsigned int i;						\
+								\
+	for_each_test(i, test) {				\
+		const typeof(test[0]) *t = &test[i];		\
+		type val;					\
+		int rv;						\
+								\
+		val = 113;					\
+		rv = parse_integer(t->str, t->base, &val);	\
+		if (rv >= 0 || val != 113) {			\
+			WARN(1, "str '%s', base %u, expected -E, got %d/"fmt"\n",\
+				t->str, t->base, rv, val);	\
+		}						\
+	}							\
+}
+
+DEFINE_TEST_OK(unsigned long long, struct test_ull, test_ull_ok)
+{
+	{"0",	10,	1,	0},
+	{"1",	10,	1,	1},
+	{"2",	10,	1,	2},
+	{"3",	10,	1,	3},
+	{"4",	10,	1,	4},
+	{"5",	10,	1,	5},
+	{"6",	10,	1,	6},
+	{"7",	10,	1,	7},
+	{"8",	10,	1,	8},
+	{"9",	10,	1,	9},
+
+	{"0",	8,	1,	0},
+	{"1",	8,	1,	1},
+	{"2",	8,	1,	2},
+	{"3",	8,	1,	3},
+	{"4",	8,	1,	4},
+	{"5",	8,	1,	5},
+	{"6",	8,	1,	6},
+	{"7",	8,	1,	7},
+
+	{"0",	16,	1,	0},
+	{"1",	16,	1,	1},
+	{"2",	16,	1,	2},
+	{"3",	16,	1,	3},
+	{"4",	16,	1,	4},
+	{"5",	16,	1,	5},
+	{"6",	16,	1,	6},
+	{"7",	16,	1,	7},
+	{"8",	16,	1,	8},
+	{"9",	16,	1,	9},
+	{"a",	16,	1,	10},
+	{"b",	16,	1,	11},
+	{"c",	16,	1,	12},
+	{"d",	16,	1,	13},
+	{"e",	16,	1,	14},
+	{"f",	16,	1,	15},
+	{"A",	16,	1,	10},
+	{"B",	16,	1,	11},
+	{"C",	16,	1,	12},
+	{"D",	16,	1,	13},
+	{"E",	16,	1,	14},
+	{"F",	16,	1,	15},
+
+	{"127",				10,	3,	127},
+	{"128",				10,	3,	128},
+	{"255",				10,	3,	255},
+	{"256",				10,	3,	256},
+	{"32767",			10,	5,	32767},
+	{"32768",			10,	5,	32768},
+	{"65535",			10,	5,	65535},
+	{"65536",			10,	5,	65536},
+	{"2147483647",			10,	10,	2147483647},
+	{"2147483648",			10,	10,	2147483648ull},
+	{"4294967295",			10,	10,	4294967295ull},
+	{"4294967296",			10,	10,	4294967296},
+	{"9223372036854775807",		10,	19,	9223372036854775807},
+	{"9223372036854775808",		10,	19,	9223372036854775808ull},
+	{"18446744073709551615",	10,	20,	18446744073709551615ull},
+
+	{"177",				8,	3,	0177},
+	{"200",				8,	3,	0200},
+	{"377",				8,	3,	0377},
+	{"400",				8,	3,	0400},
+	{"77777",			8,	5,	077777},
+	{"100000",			8,	6,	0100000},
+	{"177777",			8,	6,	0177777},
+	{"200000",			8,	6,	0200000},
+	{"17777777777",			8,	11,	017777777777},
+	{"20000000000",			8,	11,	020000000000},
+	{"37777777777",			8,	11,	037777777777},
+	{"40000000000",			8,	11,	040000000000},
+	{"777777777777777777777",	8,	21,	0777777777777777777777},
+	{"1000000000000000000000",	8,	22,	01000000000000000000000},
+	{"1777777777777777777777",	8,	22,	01777777777777777777777},
+
+	{"7f",			16,	2,	0x7f},
+	{"80",			16,	2,	0x80},
+	{"ff",			16,	2,	0xff},
+	{"100",			16,	3,	0x100},
+	{"7fff",		16,	4,	0x7fff},
+	{"8000",		16,	4,	0x8000},
+	{"ffff",		16,	4,	0xffff},
+	{"10000",		16,	5,	0x10000},
+	{"7fffffff",		16,	8,	0x7fffffff},
+	{"80000000",		16,	8,	0x80000000},
+	{"ffffffff",		16,	8,	0xffffffff},
+	{"100000000",		16,	9,	0x100000000},
+	{"7fffffffffffffff",	16,	16,	0x7fffffffffffffff},
+	{"8000000000000000",	16,	16,	0x8000000000000000},
+	{"ffffffffffffffff",	16,	16,	0xffffffffffffffff},
+	/* test sign */
+	{"+0",	10,	2,	0},
+	{"+42",	10,	3,	42},
+	/* test termination */
+	{"42/",	10,	2,	42},
+	{"42:",	10,	2,	42},
+	{"42/",	8,	2,	042},
+	{"428",	8,	2,	042},
+	{"42/",	16,	2,	0x42},
+	{"42`",	16,	2,	0x42},
+	{"42g",	16,	2,	0x42},
+	{"42@",	16,	2,	0x42},
+	{"42G",	16,	2,	0x42},
+	/* base autodetection */
+	{"010",		0,	3,	8},
+	{"0x10",	0,	4,	16},
+	{"0X10",	0,	4,	16},
+};
+
+static void __init test_parse_integer_ull_ok(void)
+{
+	TEST_OK(unsigned long long, "%llu", test_ull_ok);
+}
+
+DEFINE_TEST_FAIL(unsigned long long, test_ull_fail)
+{
+	/* type overflow */
+	{"10000000000000000000000000000000000000000000000000000000000000000",	2},
+	{"18446744073709551616",	10},
+	{"2000000000000000000000",	8},
+	{"10000000000000000",		16},
+
+	{"",	0},
+	{"",	10},
+	{"",	8},
+	{"",	16},
+	{"+",	0},
+	{"+",	10},
+	{"+",	8},
+	{"+",	16},
+	{"-",	0},
+	{"-",	10},
+	{"-",	8},
+	{"-",	16},
+	{" ",	0},
+	{" ",	10},
+	{" ",	8},
+	{" ",	16},
+	{"\n",	0},
+	{"\n",	10},
+	{"\n",	8},
+	{"\n",	16},
+	{" 0",	0},
+	{" 0",	10},
+	{" 0",	8},
+	{" 0",	16},
+	{"\n0",	0},
+	{"\n0",	10},
+	{"\n0",	8},
+	{"\n0",	16},
+	/* non-digit */
+	{"/",	10},
+	{":",	10},
+	{"/",	8},
+	{"8",	8},
+	{"/",	16},
+	{":",	16},
+	{"`",	16},
+	{"g",	16},
+	{"@",	16},
+	{"G",	16},
+	{"/0",	10},
+	{":0",	10},
+	{"/0",	8},
+	{"80",	8},
+	{"/0",	16},
+	{":0",	16},
+	{"`0",	16},
+	{"g0",	16},
+	{"@0",	16},
+	{"G0",	16},
+
+	{"-0",	0},
+	{"-0",	10},
+	{"-0",	8},
+	{"-0",	16},
+	{"-1",	0},
+	{"-1",	10},
+	{"-1",	8},
+	{"-1",	16},
+	/* accept only one sign */
+	{"--",	0},
+	{"--",	10},
+	{"--",	8},
+	{"--",	16},
+	{"-+",	0},
+	{"-+",	10},
+	{"-+",	8},
+	{"-+",	16},
+	{"+-",	0},
+	{"+-",	10},
+	{"+-",	8},
+	{"+-",	16},
+	{"++",	0},
+	{"++",	10},
+	{"++",	8},
+	{"++",	16},
+	{"--0",	0},
+	{"--0",	10},
+	{"--0",	8},
+	{"--0",	16},
+	{"-+0",	0},
+	{"-+0",	10},
+	{"-+0",	8},
+	{"-+0",	16},
+	{"+-0",	0},
+	{"+-0",	10},
+	{"+-0",	8},
+	{"+-0",	16},
+	{"++0",	0},
+	{"++0",	10},
+	{"++0",	8},
+	{"++0",	16},
+};
+
+static void __init test_parse_integer_ull_fail(void)
+{
+	TEST_FAIL(unsigned long long, "%llu", test_ull_fail);
+}
+
+DEFINE_TEST_OK(long long, struct test_ll, test_ll_ok)
+{
+	{"-9223372036854775808",10,	20,	LLONG_MIN},
+	{"-4294967296",		10,	11,	-4294967296},
+	{"-2147483648",		10,	11,	-2147483648ll},
+	{"-65536",		10,	6,	-65536},
+	{"-32768",		10,	6,	-32768},
+	{"-256",		10,	4,	-256},
+	{"-128",		10,	4,	-128},
+	{"-0",			10,	2,	0},
+	{"0",			10,	1,	0},
+	{"127",			10,	3,	127},
+	{"255",			10,	3,	255},
+	{"32767",		10,	5,	32767},
+	{"65535",		10,	5,	65535},
+	{"2147483647",		10,	10,	2147483647},
+	{"4294967295",		10,	10,	4294967295ll},
+	{"9223372036854775807",	10,	19,	9223372036854775807},
+};
+
+static void __init test_parse_integer_ll_ok(void)
+{
+	TEST_OK(long long, "%lld", test_ll_ok);
+}
+
+DEFINE_TEST_FAIL(long long, test_ll_fail)
+{
+	{"-9223372036854775809",	10},
+	{"9223372036854775808",		10},
+};
+
+static void __init test_parse_integer_ll_fail(void)
+{
+	TEST_FAIL(long long, "%lld", test_ll_fail);
+}
+
+DEFINE_TEST_OK(unsigned int, struct test_u, test_u_ok)
+{
+	{"0",		10,	1,	0},
+	{"127",		10,	3,	127},
+	{"128",		10,	3,	128},
+	{"255",		10,	3,	255},
+	{"256",		10,	3,	256},
+	{"32767",	10,	5,	32767},
+	{"32768",	10,	5,	32768},
+	{"65535",	10,	5,	65535},
+	{"65536",	10,	5,	65536},
+	{"2147483647",	10,	10,	2147483647},
+	{"2147483648",	10,	10,	2147483648u},
+	{"4294967295",	10,	10,	4294967295u},
+};
+
+static void __init test_parse_integer_u_ok(void)
+{
+	TEST_OK(unsigned int, "%u", test_u_ok);
+}
+
+DEFINE_TEST_FAIL(unsigned int, test_u_fail)
+{
+	{"4294967296",			10},
+	{"9223372036854775807",		10},
+	{"9223372036854775808",		10},
+	{"18446744073709551615",	10},
+};
+
+static void __init test_parse_integer_u_fail(void)
+{
+	TEST_FAIL(unsigned int, "%u", test_u_fail);
+}
+
+DEFINE_TEST_OK(int, struct test_i, test_i_ok)
+{
+	{"-2147483648",	10,	11,	INT_MIN},
+	{"-65536",	10,	6,	-65536},
+	{"-32768",	10,	6,	-32768},
+	{"-256",	10,	4,	-256},
+	{"-128",	10,	4,	-128},
+	{"-0",		10,	2,	0},
+	{"0",		10,	1,	0},
+	{"127",		10,	3,	127},
+	{"255",		10,	3,	255},
+	{"32767",	10,	5,	32767},
+	{"65535",	10,	5,	65535},
+	{"2147483647",	10,	10,	2147483647},
+};
+
+static void __init test_parse_integer_i_ok(void)
+{
+	TEST_OK(int, "%d", test_i_ok);
+}
+
+DEFINE_TEST_FAIL(int, test_i_fail)
+{
+	{"-9223372036854775809",	10},
+	{"-9223372036854775808",	10},
+	{"-4294967296",			10},
+	{"-2147483649",			10},
+	{"2147483648",			10},
+	{"4294967295",			10},
+	{"9223372036854775807",		10},
+	{"9223372036854775808",		10},
+};
+
+static void __init test_parse_integer_i_fail(void)
+{
+	TEST_FAIL(int, "%d", test_i_fail);
+}
+
+DEFINE_TEST_OK(unsigned short, struct test_us, test_us_ok)
+{
+	{"0",		10,	1,	0},
+	{"127",		10,	3,	127},
+	{"128",		10,	3,	128},
+	{"255",		10,	3,	255},
+	{"256",		10,	3,	256},
+	{"32767",	10,	5,	32767},
+	{"32768",	10,	5,	32768},
+	{"65535",	10,	5,	65535},
+};
+
+static void __init test_parse_integer_us_ok(void)
+{
+	TEST_OK(unsigned short, "%hu", test_us_ok);
+}
+
+DEFINE_TEST_FAIL(unsigned short, test_us_fail)
+{
+	{"65536",			10},
+	{"2147483647",			10},
+	{"2147483648",			10},
+	{"4294967295",			10},
+	{"4294967296",			10},
+	{"9223372036854775807",		10},
+	{"9223372036854775808",		10},
+	{"18446744073709551615",	10},
+};
+
+static void __init test_parse_integer_us_fail(void)
+{
+	TEST_FAIL(unsigned short, "%hu", test_us_fail);
+}
+
+DEFINE_TEST_OK(short, struct test_s, test_s_ok)
+{
+	{"-32768",	10,	6,	-32768},
+	{"-256",	10,	4,	-256},
+	{"-128",	10,	4,	-128},
+	{"-0",		10,	2,	0},
+	{"0",		10,	1,	0},
+	{"127",		10,	3,	127},
+	{"255",		10,	3,	255},
+	{"32767",	10,	5,	32767},
+};
+
+static void __init test_parse_integer_s_ok(void)
+{
+	TEST_OK(short, "%hd", test_s_ok);
+}
+
+DEFINE_TEST_FAIL(short, test_s_fail)
+{
+	{"-9223372036854775809",	10},
+	{"-9223372036854775808",	10},
+	{"-4294967296",			10},
+	{"-2147483649",			10},
+	{"-2147483648",			10},
+	{"-65536",			10},
+	{"-32769",			10},
+	{"32768",			10},
+	{"65535",			10},
+	{"2147483647",			10},
+	{"2147483648",			10},
+	{"4294967295",			10},
+	{"9223372036854775807",		10},
+	{"9223372036854775808",		10},
+};
+
+static void __init test_parse_integer_s_fail(void)
+{
+	TEST_FAIL(short, "%hd", test_s_fail);
+}
+
+DEFINE_TEST_OK(unsigned char, struct test_uc, test_uc_ok)
+{
+	{"0",	10,	1,	0},
+	{"127",	10,	3,	127},
+	{"128",	10,	3,	128},
+	{"255",	10,	3,	255},
+};
+
+static void __init test_parse_integer_uc_ok(void)
+{
+	TEST_OK(unsigned char, "%hhu", test_uc_ok);
+}
+
+DEFINE_TEST_FAIL(unsigned char, test_uc_fail)
+{
+	{"256",				10},
+	{"32767",			10},
+	{"32768",			10},
+	{"65535",			10},
+	{"65536",			10},
+	{"2147483647",			10},
+	{"2147483648",			10},
+	{"4294967295",			10},
+	{"4294967296",			10},
+	{"9223372036854775807",		10},
+	{"9223372036854775808",		10},
+	{"18446744073709551615",	10},
+};
+
+static void __init test_parse_integer_uc_fail(void)
+{
+	TEST_FAIL(unsigned char, "%hhu", test_uc_fail);
+}
+
+DEFINE_TEST_OK(signed char, struct test_sc, test_sc_ok)
+{
+	{"-128",	10,	4,	-128},
+	{"-0",		10,	2,	0},
+	{"0",		10,	1,	0},
+	{"127",		10,	3,	127},
+};
+
+static void __init test_parse_integer_sc_ok(void)
+{
+	TEST_OK(signed char, "%hhd", test_sc_ok);
+}
+
+DEFINE_TEST_FAIL(signed char, test_sc_fail)
+{
+	{"-9223372036854775809",	10},
+	{"-9223372036854775808",	10},
+	{"-4294967296",			10},
+	{"-2147483649",			10},
+	{"-2147483648",			10},
+	{"-65536",			10},
+	{"-32769",			10},
+	{"-32768",			10},
+	{"-256",			10},
+	{"-129",			10},
+	{"128",				10},
+	{"255",				10},
+	{"32767",			10},
+	{"32768",			10},
+	{"65535",			10},
+	{"2147483647",			10},
+	{"2147483648",			10},
+	{"4294967295",			10},
+	{"9223372036854775807",		10},
+	{"9223372036854775808",		10},
+};
+
+static void __init test_parse_integer_sc_fail(void)
+{
+	TEST_FAIL(signed char, "%hhd", test_sc_fail);
+}
+
+static int __init test_parse_integer_init(void)
+{
+	test_parse_integer_ull_ok();
+	test_parse_integer_ull_fail();
+	test_parse_integer_ll_ok();
+	test_parse_integer_ll_fail();
+	test_parse_integer_u_ok();
+	test_parse_integer_u_fail();
+	test_parse_integer_i_ok();
+	test_parse_integer_i_fail();
+	test_parse_integer_us_ok();
+	test_parse_integer_us_fail();
+	test_parse_integer_s_ok();
+	test_parse_integer_s_fail();
+	test_parse_integer_uc_ok();
+	test_parse_integer_uc_fail();
+	test_parse_integer_sc_ok();
+	test_parse_integer_sc_fail();
+	return -EINVAL;
+}
+module_init(test_parse_integer_init);
+MODULE_LICENSE("Dual BSD/GPL");
-- 
2.0.4

[PATCH 04/12] parse-integer: rewrite kstrto*()

[Alexey Dobriyan]

Rewrite kstrto*() functions through parse_integer().

_kstrtoul() and _kstrtol() are removed because parse_integer()
can dispatch based on sizeof(long) saving function call.

Also move function definitions and comment one instance.
Remove redundant boilerplate comments from elsewhere.

High bit base hack suggested by Andrew Morton.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 include/linux/kernel.h        | 124 -----------------------
 include/linux/parse-integer.h | 109 +++++++++++++++++++++
 lib/kstrtox.c                 | 222 ------------------------------------------
 lib/parse-integer.c           |  26 ++++-
 4 files changed, 134 insertions(+), 347 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 9d3f543..5c36063 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -264,130 +264,6 @@ void do_exit(long error_code)
 void complete_and_exit(struct completion *, long)
 	__noreturn;
 
-/* Internal, do not use. */
-int __must_check _kstrtoul(const char *s, unsigned int base, unsigned long *res);
-int __must_check _kstrtol(const char *s, unsigned int base, long *res);
-
-int __must_check kstrtoull(const char *s, unsigned int base, unsigned long long *res);
-int __must_check kstrtoll(const char *s, unsigned int base, long long *res);
-
-/**
- * kstrtoul - convert a string to an unsigned long
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign, but not a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
-*/
-static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
-{
-	/*
-	 * We want to shortcut function call, but
-	 * __builtin_types_compatible_p(unsigned long, unsigned long long) = 0.
-	 */
-	if (sizeof(unsigned long) == sizeof(unsigned long long) &&
-	    __alignof__(unsigned long) == __alignof__(unsigned long long))
-		return kstrtoull(s, base, (unsigned long long *)res);
-	else
-		return _kstrtoul(s, base, res);
-}
-
-/**
- * kstrtol - convert a string to a long
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign or a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
- */
-static inline int __must_check kstrtol(const char *s, unsigned int base, long *res)
-{
-	/*
-	 * We want to shortcut function call, but
-	 * __builtin_types_compatible_p(long, long long) = 0.
-	 */
-	if (sizeof(long) == sizeof(long long) &&
-	    __alignof__(long) == __alignof__(long long))
-		return kstrtoll(s, base, (long long *)res);
-	else
-		return _kstrtol(s, base, res);
-}
-
-int __must_check kstrtouint(const char *s, unsigned int base, unsigned int *res);
-int __must_check kstrtoint(const char *s, unsigned int base, int *res);
-
-static inline int __must_check kstrtou64(const char *s, unsigned int base, u64 *res)
-{
-	return kstrtoull(s, base, res);
-}
-
-static inline int __must_check kstrtos64(const char *s, unsigned int base, s64 *res)
-{
-	return kstrtoll(s, base, res);
-}
-
-static inline int __must_check kstrtou32(const char *s, unsigned int base, u32 *res)
-{
-	return kstrtouint(s, base, res);
-}
-
-static inline int __must_check kstrtos32(const char *s, unsigned int base, s32 *res)
-{
-	return kstrtoint(s, base, res);
-}
-
-int __must_check kstrtou16(const char *s, unsigned int base, u16 *res);
-int __must_check kstrtos16(const char *s, unsigned int base, s16 *res);
-int __must_check kstrtou8(const char *s, unsigned int base, u8 *res);
-int __must_check kstrtos8(const char *s, unsigned int base, s8 *res);
-
-int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
-int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
-int __must_check kstrtoul_from_user(const char __user *s, size_t count, unsigned int base, unsigned long *res);
-int __must_check kstrtol_from_user(const char __user *s, size_t count, unsigned int base, long *res);
-int __must_check kstrtouint_from_user(const char __user *s, size_t count, unsigned int base, unsigned int *res);
-int __must_check kstrtoint_from_user(const char __user *s, size_t count, unsigned int base, int *res);
-int __must_check kstrtou16_from_user(const char __user *s, size_t count, unsigned int base, u16 *res);
-int __must_check kstrtos16_from_user(const char __user *s, size_t count, unsigned int base, s16 *res);
-int __must_check kstrtou8_from_user(const char __user *s, size_t count, unsigned int base, u8 *res);
-int __must_check kstrtos8_from_user(const char __user *s, size_t count, unsigned int base, s8 *res);
-
-static inline int __must_check kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)
-{
-	return kstrtoull_from_user(s, count, base, res);
-}
-
-static inline int __must_check kstrtos64_from_user(const char __user *s, size_t count, unsigned int base, s64 *res)
-{
-	return kstrtoll_from_user(s, count, base, res);
-}
-
-static inline int __must_check kstrtou32_from_user(const char __user *s, size_t count, unsigned int base, u32 *res)
-{
-	return kstrtouint_from_user(s, count, base, res);
-}
-
-static inline int __must_check kstrtos32_from_user(const char __user *s, size_t count, unsigned int base, s32 *res)
-{
-	return kstrtoint_from_user(s, count, base, res);
-}
-
 /*
  * Obsolete, do not use.
  * Use parse_integer(), kstrto*(), kstrto*_from_user(), sscanf().
diff --git a/include/linux/parse-integer.h b/include/linux/parse-integer.h
index 5dd0347..ba620cd 100644
--- a/include/linux/parse-integer.h
+++ b/include/linux/parse-integer.h
@@ -76,4 +76,113 @@ int _parse_integer_ll(const char *s, unsigned int base, long long *val);
 int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val);
 void _parse_integer_link_time_error(void);
 const char *_parse_integer_fixup_radix(const char *s, unsigned int *base);
+#define PARSE_INTEGER_NEWLINE 0x80000000u
+
+/*
+ * Convert integer string representation terminated by \n\0 or \0 to an integer.
+ *
+ * Return 0 on success or -E.
+ *
+ * See parse_integer().
+ */
+static inline int __must_check kstrtoull(const char *s, unsigned int base, unsigned long long *res)
+{
+	return parse_integer(s, base | PARSE_INTEGER_NEWLINE, res);
+}
+
+static inline int __must_check kstrtoll(const char *s, unsigned int base, long long *res)
+{
+	return parse_integer(s, base | PARSE_INTEGER_NEWLINE, res);
+}
+
+static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
+{
+	return parse_integer(s, base | PARSE_INTEGER_NEWLINE, res);
+}
+
+static inline int __must_check kstrtol(const char *s, unsigned int base, long *res)
+{
+	return parse_integer(s, base | PARSE_INTEGER_NEWLINE, res);
+}
+
+static inline int __must_check kstrtouint(const char *s, unsigned int base, unsigned int *res)
+{
+	return parse_integer(s, base | PARSE_INTEGER_NEWLINE, res);
+}
+
+static inline int __must_check kstrtoint(const char *s, unsigned int base, int *res)
+{
+	return parse_integer(s, base | PARSE_INTEGER_NEWLINE, res);
+}
+
+static inline int __must_check kstrtou64(const char *s, unsigned int base, u64 *res)
+{
+	return kstrtoull(s, base, res);
+}
+
+static inline int __must_check kstrtos64(const char *s, unsigned int base, s64 *res)
+{
+	return kstrtoll(s, base, res);
+}
+
+static inline int __must_check kstrtou32(const char *s, unsigned int base, u32 *res)
+{
+	return kstrtouint(s, base, res);
+}
+
+static inline int __must_check kstrtos32(const char *s, unsigned int base, s32 *res)
+{
+	return kstrtoint(s, base, res);
+}
+
+static inline int __must_check kstrtou16(const char *s, unsigned int base, u16 *res)
+{
+	return parse_integer(s, base | PARSE_INTEGER_NEWLINE, res);
+}
+
+static inline int __must_check kstrtos16(const char *s, unsigned int base, s16 *res)
+{
+	return parse_integer(s, base | PARSE_INTEGER_NEWLINE, res);
+}
+
+static inline int __must_check kstrtou8(const char *s, unsigned int base, u8 *res)
+{
+	return parse_integer(s, base | PARSE_INTEGER_NEWLINE, res);
+}
+
+static inline int __must_check kstrtos8(const char *s, unsigned int base, s8 *res)
+{
+	return parse_integer(s, base | PARSE_INTEGER_NEWLINE, res);
+}
+
+int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
+int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
+int __must_check kstrtoul_from_user(const char __user *s, size_t count, unsigned int base, unsigned long *res);
+int __must_check kstrtol_from_user(const char __user *s, size_t count, unsigned int base, long *res);
+int __must_check kstrtouint_from_user(const char __user *s, size_t count, unsigned int base, unsigned int *res);
+int __must_check kstrtoint_from_user(const char __user *s, size_t count, unsigned int base, int *res);
+int __must_check kstrtou16_from_user(const char __user *s, size_t count, unsigned int base, u16 *res);
+int __must_check kstrtos16_from_user(const char __user *s, size_t count, unsigned int base, s16 *res);
+int __must_check kstrtou8_from_user(const char __user *s, size_t count, unsigned int base, u8 *res);
+int __must_check kstrtos8_from_user(const char __user *s, size_t count, unsigned int base, s8 *res);
+
+static inline int __must_check kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)
+{
+	return kstrtoull_from_user(s, count, base, res);
+}
+
+static inline int __must_check kstrtos64_from_user(const char __user *s, size_t count, unsigned int base, s64 *res)
+{
+	return kstrtoll_from_user(s, count, base, res);
+}
+
+static inline int __must_check kstrtou32_from_user(const char __user *s, size_t count, unsigned int base, u32 *res)
+{
+	return kstrtouint_from_user(s, count, base, res);
+}
+
+static inline int __must_check kstrtos32_from_user(const char __user *s, size_t count, unsigned int base, s32 *res)
+{
+	return kstrtoint_from_user(s, count, base, res);
+}
 #endif
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index 652fd6b..1698b28 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -67,228 +67,6 @@ unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long
 	return rv;
 }
 
-/**
- * kstrtoull - convert a string to an unsigned long long
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign, but not a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
- */
-int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
-{
-	unsigned long long _res;
-	int rv;
-
-	rv = parse_integer(s, base, &_res);
-	if (rv < 0)
-		return rv;
-	s += rv;
-	if (*s == '\n')
-		s++;
-	if (*s)
-		return -EINVAL;
-	*res = _res;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtoull);
-
-/**
- * kstrtoll - convert a string to a long long
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign or a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
- */
-int kstrtoll(const char *s, unsigned int base, long long *res)
-{
-	long long _res;
-	int rv;
-
-	rv = parse_integer(s, base, &_res);
-	if (rv < 0)
-		return rv;
-	s += rv;
-	if (*s == '\n')
-		s++;
-	if (*s)
-		return -EINVAL;
-	*res = _res;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtoll);
-
-/* Internal, do not use. */
-int _kstrtoul(const char *s, unsigned int base, unsigned long *res)
-{
-	unsigned long long tmp;
-	int rv;
-
-	rv = kstrtoull(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (unsigned long long)(unsigned long)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(_kstrtoul);
-
-/* Internal, do not use. */
-int _kstrtol(const char *s, unsigned int base, long *res)
-{
-	long long tmp;
-	int rv;
-
-	rv = kstrtoll(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (long long)(long)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(_kstrtol);
-
-/**
- * kstrtouint - convert a string to an unsigned int
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign, but not a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
- */
-int kstrtouint(const char *s, unsigned int base, unsigned int *res)
-{
-	unsigned long long tmp;
-	int rv;
-
-	rv = kstrtoull(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (unsigned long long)(unsigned int)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtouint);
-
-/**
- * kstrtoint - convert a string to an int
- * @s: The start of the string. The string must be null-terminated, and may also
- *  include a single newline before its terminating null. The first character
- *  may also be a plus sign or a minus sign.
- * @base: The number base to use. The maximum supported base is 16. If base is
- *  given as 0, then the base of the string is automatically detected with the
- *  conventional semantics - If it begins with 0x the number will be parsed as a
- *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
- *  parsed as an octal number. Otherwise it will be parsed as a decimal.
- * @res: Where to write the result of the conversion on success.
- *
- * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
- */
-int kstrtoint(const char *s, unsigned int base, int *res)
-{
-	long long tmp;
-	int rv;
-
-	rv = kstrtoll(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (long long)(int)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtoint);
-
-int kstrtou16(const char *s, unsigned int base, u16 *res)
-{
-	unsigned long long tmp;
-	int rv;
-
-	rv = kstrtoull(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (unsigned long long)(u16)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtou16);
-
-int kstrtos16(const char *s, unsigned int base, s16 *res)
-{
-	long long tmp;
-	int rv;
-
-	rv = kstrtoll(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (long long)(s16)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtos16);
-
-int kstrtou8(const char *s, unsigned int base, u8 *res)
-{
-	unsigned long long tmp;
-	int rv;
-
-	rv = kstrtoull(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (unsigned long long)(u8)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtou8);
-
-int kstrtos8(const char *s, unsigned int base, s8 *res)
-{
-	long long tmp;
-	int rv;
-
-	rv = kstrtoll(s, base, &tmp);
-	if (rv < 0)
-		return rv;
-	if (tmp != (long long)(s8)tmp)
-		return -ERANGE;
-	*res = tmp;
-	return 0;
-}
-EXPORT_SYMBOL(kstrtos8);
-
 #define kstrto_from_user(f, g, type)					\
 int f(const char __user *s, size_t count, unsigned int base, type *res)	\
 {									\
diff --git a/lib/parse-integer.c b/lib/parse-integer.c
index eb24307..7c7f48b 100644
--- a/lib/parse-integer.c
+++ b/lib/parse-integer.c
@@ -31,7 +31,7 @@ const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
 	return s;
 }
 
-static int __parse_integer(const char *s, unsigned int base, unsigned long long *val)
+static int ___parse_integer(const char *s, unsigned int base, unsigned long long *val)
 {
 	const char *s0 = s, *sd;
 	unsigned long long acc;
@@ -63,6 +63,26 @@ static int __parse_integer(const char *s, unsigned int base, unsigned long long
 	return s - s0;
 }
 
+static int __parse_integer(const char *s, unsigned int base, unsigned long long *val)
+{
+	unsigned long long tmp;
+	int rv;
+
+	rv = ___parse_integer(s, base & ~PARSE_INTEGER_NEWLINE, &tmp);
+	if (rv < 0)
+		return rv;
+	if (base & PARSE_INTEGER_NEWLINE) {
+		/* Accept "integer\0" or "integer\n\0" */
+		s += rv;
+		if (*s == '\n')
+			s++;
+		if (*s)
+			return -EINVAL;
+	}
+	*val = tmp;
+	return rv;
+}
+
 int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val)
 {
 	char sign;
@@ -77,6 +97,8 @@ int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val
 	rv = __parse_integer(s, base, val);
 	if (rv < 0)
 		return rv;
+	if (base & PARSE_INTEGER_NEWLINE)
+		return 0;
 	return rv + !!sign;
 }
 EXPORT_SYMBOL(_parse_integer_ull);
@@ -103,6 +125,8 @@ int _parse_integer_ll(const char *s, unsigned int base, long long *val)
 			return -ERANGE;
 		*val = tmp;
 	}
+	if (base & PARSE_INTEGER_NEWLINE)
+		return 0;
 	return rv + !!sign;
 }
 EXPORT_SYMBOL(_parse_integer_ll);
-- 
2.0.4

[PATCH 05/12] parse_integer: convert scanf()

[Alexey Dobriyan]

Remove base second guessing -- done by parse_integer() just fine.

Uniformly fix too liberal acceptance in %lu/%ld cases in the next patch.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 lib/vsprintf.c | 36 ++++++++++--------------------------
 1 file changed, 10 insertions(+), 26 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index da39c60..6509c54 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2470,8 +2470,6 @@ EXPORT_SYMBOL_GPL(bprintf);
 int vsscanf(const char *buf, const char *fmt, va_list args)
 {
 	const char *str = buf;
-	char *next;
-	char digit;
 	int num = 0;
 	u8 qualifier;
 	unsigned int base;
@@ -2483,6 +2481,8 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
 	bool is_sign;
 
 	while (*fmt) {
+		int len;
+
 		/* skip any white space in format */
 		/* white space in format matchs any amount of
 		 * white space, including none, in the input.
@@ -2611,35 +2611,22 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
 		 */
 		str = skip_spaces(str);
 
-		digit = *str;
-		if (is_sign && digit == '-')
-			digit = *(str + 1);
-
-		if (!digit
-		    || (base == 16 && !isxdigit(digit))
-		    || (base == 10 && !isdigit(digit))
-		    || (base == 8 && (!isdigit(digit) || digit > '7'))
-		    || (base == 0 && !isdigit(digit)))
-			break;
-
 		if (is_sign)
-			val.s = qualifier != 'L' ?
-				simple_strtol(str, &next, base) :
-				simple_strtoll(str, &next, base);
+			len = parse_integer(str, base, &val.s);
 		else
-			val.u = qualifier != 'L' ?
-				simple_strtoul(str, &next, base) :
-				simple_strtoull(str, &next, base);
+			len = parse_integer(str, base, &val.u);
+		if (len < 0)
+			break;
 
-		if (field_width > 0 && next - str > field_width) {
+		if (field_width > 0) {
 			if (base == 0)
 				_parse_integer_fixup_radix(str, &base);
-			while (next - str > field_width) {
+			while (len > field_width) {
 				if (is_sign)
 					val.s = div_s64(val.s, base);
 				else
 					val.u = div_u64(val.u, base);
-				--next;
+				len--;
 			}
 		}
 
@@ -2680,10 +2667,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
 			break;
 		}
 		num++;
-
-		if (!next)
-			break;
-		str = next;
+		str += len;
 	}
 
 	return num;
-- 
2.0.4

[PATCH 06/12] scanf: fix type range overflow

[Alexey Dobriyan]

Fun fact:

	uint8_t val;
	sscanf("256", "%hhu", &val);

will return 1 and make val=0 (clearly bogus).

Userspace sscanf() reports 1 parsed value, returns incorrect value
but sets errno to ERANGE only for "%u" conversion and higher.
%hhu and %hu are left in the cold.

Having no way to report errno=ERANGE in kernel, don't report
successful parsing.

Patch allows to remove checks and switch to proper types
in several (most?) cases:

	grep -e 'scanf.*%[0-9]\+[dioux]' -n -r .

Such checks can be incorrect too -- checking for 3 digits with %3u
for parsing uint8_t is not enough.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 lib/vsprintf.c | 45 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 6509c54..58051b4 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2632,44 +2632,67 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
 
 		switch (qualifier) {
 		case 'H':	/* that's 'hh' in format */
-			if (is_sign)
+			if (is_sign) {
+				if (val.s != (signed char)val.s)
+					goto out;
 				*va_arg(args, signed char *) = val.s;
-			else
+			} else {
+				if (val.u != (unsigned char)val.u)
+					goto out;
 				*va_arg(args, unsigned char *) = val.u;
+			}
 			break;
 		case 'h':
-			if (is_sign)
+			if (is_sign) {
+				if (val.s != (short)val.s)
+					goto out;
 				*va_arg(args, short *) = val.s;
-			else
+			} else {
+				if (val.u != (unsigned short)val.u)
+					goto out;
 				*va_arg(args, unsigned short *) = val.u;
+			}
 			break;
 		case 'l':
-			if (is_sign)
+			if (is_sign) {
+				if (val.s != (long)val.s)
+					goto out;
 				*va_arg(args, long *) = val.s;
-			else
+			} else {
+				if (val.u != (unsigned long)val.u)
+					goto out;
 				*va_arg(args, unsigned long *) = val.u;
+			}
 			break;
 		case 'L':
-			if (is_sign)
+			if (is_sign) {
 				*va_arg(args, long long *) = val.s;
-			else
+			} else {
 				*va_arg(args, unsigned long long *) = val.u;
+			}
 			break;
 		case 'Z':
 		case 'z':
+			if (val.u != (size_t)val.u)
+				goto out;
 			*va_arg(args, size_t *) = val.u;
 			break;
 		default:
-			if (is_sign)
+			if (is_sign) {
+				if (val.s != (int)val.s)
+					goto out;
 				*va_arg(args, int *) = val.s;
-			else
+			} else {
+				if (val.u != (unsigned int)val.u)
+					goto out;
 				*va_arg(args, unsigned int *) = val.u;
+			}
 			break;
 		}
 		num++;
 		str += len;
 	}
-
+out:
 	return num;
 }
 EXPORT_SYMBOL(vsscanf);
-- 
2.0.4

[PATCH 07/12] parse_integer: convert lib/

[Alexey Dobriyan]

Convert away lib/ from deprecated simple_strto*() interfaces.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 lib/cmdline.c | 44 ++++++++++++++++++++++++--------------------
 lib/parser.c  | 33 ++++++++++++++++-----------------
 lib/swiotlb.c |  2 +-
 3 files changed, 41 insertions(+), 38 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 8f13cf7..c248c58 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -27,7 +27,7 @@ static int get_range(char **str, int *pint)
 	int x, inc_counter, upper_range;
 
 	(*str)++;
-	upper_range = simple_strtol((*str), NULL, 0);
+	parse_integer(*str, 0, &upper_range);
 	inc_counter = upper_range - *pint;
 	for (x = *pint; x < upper_range; x++)
 		*pint++ = x;
@@ -51,13 +51,14 @@ static int get_range(char **str, int *pint)
 
 int get_option(char **str, int *pint)
 {
-	char *cur = *str;
+	int len;
 
-	if (!cur || !(*cur))
+	if (!str || !*str)
 		return 0;
-	*pint = simple_strtol(cur, str, 0);
-	if (cur == *str)
+	len = parse_integer(*str, 0, pint);
+	if (len < 0)
 		return 0;
+	*str += len;
 	if (**str == ',') {
 		(*str)++;
 		return 2;
@@ -126,38 +127,41 @@ EXPORT_SYMBOL(get_options);
 
 unsigned long long memparse(const char *ptr, char **retptr)
 {
-	char *endptr;	/* local pointer to end of parsed string */
-
-	unsigned long long ret = simple_strtoull(ptr, &endptr, 0);
-
-	switch (*endptr) {
+	unsigned long long val = 0;
+	int len;
+
+	len = parse_integer(ptr, 0, &val);
+	if (len < 0)
+		goto out;
+	ptr += len;
+	switch (*ptr) {
 	case 'E':
 	case 'e':
-		ret <<= 10;
+		val <<= 10;
 	case 'P':
 	case 'p':
-		ret <<= 10;
+		val <<= 10;
 	case 'T':
 	case 't':
-		ret <<= 10;
+		val <<= 10;
 	case 'G':
 	case 'g':
-		ret <<= 10;
+		val <<= 10;
 	case 'M':
 	case 'm':
-		ret <<= 10;
+		val <<= 10;
 	case 'K':
 	case 'k':
-		ret <<= 10;
-		endptr++;
+		val <<= 10;
+		ptr++;
 	default:
 		break;
 	}
-
+out:
 	if (retptr)
-		*retptr = endptr;
+		*retptr = (char *)ptr;
 
-	return ret;
+	return val;
 }
 EXPORT_SYMBOL(memparse);
 
diff --git a/lib/parser.c b/lib/parser.c
index b6d1163..f003867 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -44,7 +44,7 @@ static int match_one(char *s, const char *p, substring_t args[])
 		p = meta + 1;
 
 		if (isdigit(*p))
-			len = simple_strtoul(p, (char **) &p, 10);
+			p += parse_integer(p, 10, (unsigned int *)&len);
 		else if (*p == '%') {
 			if (*s++ != '%')
 				return 0;
@@ -57,6 +57,11 @@ static int match_one(char *s, const char *p, substring_t args[])
 
 		args[argc].from = s;
 		switch (*p++) {
+			union {
+				int i;
+				unsigned int u;
+			} u;
+
 		case 's': {
 			size_t str_len = strlen(s);
 
@@ -68,19 +73,20 @@ static int match_one(char *s, const char *p, substring_t args[])
 			break;
 		}
 		case 'd':
-			simple_strtol(s, &args[argc].to, 0);
+			len = parse_integer(s, 0, &u.i);
 			goto num;
 		case 'u':
-			simple_strtoul(s, &args[argc].to, 0);
+			len = parse_integer(s, 0, &u.u);
 			goto num;
 		case 'o':
-			simple_strtoul(s, &args[argc].to, 8);
+			len = parse_integer(s, 8, &u.u);
 			goto num;
 		case 'x':
-			simple_strtoul(s, &args[argc].to, 16);
+			len = parse_integer(s, 16, &u.u);
 		num:
-			if (args[argc].to == args[argc].from)
+			if (len < 0)
 				return 0;
+			args[argc].to = args[argc].from + len;
 			break;
 		default:
 			return 0;
@@ -127,10 +133,8 @@ EXPORT_SYMBOL(match_token);
  */
 static int match_number(substring_t *s, int *result, int base)
 {
-	char *endp;
 	char *buf;
 	int ret;
-	long val;
 	size_t len = s->to - s->from;
 
 	buf = kmalloc(len + 1, GFP_KERNEL);
@@ -139,16 +143,11 @@ static int match_number(substring_t *s, int *result, int base)
 	memcpy(buf, s->from, len);
 	buf[len] = '\0';
 
-	ret = 0;
-	val = simple_strtol(buf, &endp, base);
-	if (endp == buf)
-		ret = -EINVAL;
-	else if (val < (long)INT_MIN || val > (long)INT_MAX)
-		ret = -ERANGE;
-	else
-		*result = (int) val;
+	ret = parse_integer(buf, base, result);
 	kfree(buf);
-	return ret;
+	if (ret < 0)
+		return ret;
+	return 0;
 }
 
 /**
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 4abda07..33178e5 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -100,7 +100,7 @@ static int __init
 setup_io_tlb_npages(char *str)
 {
 	if (isdigit(*str)) {
-		io_tlb_nslabs = simple_strtoul(str, &str, 0);
+		str += parse_integer(str, 0, &io_tlb_nslabs);
 		/* avoid tail segment of size < IO_TLB_SEGSIZE */
 		io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
 	}
-- 
2.0.4

[PATCH 08/12] parse_integer: convert mm/

[Alexey Dobriyan]

Convert mm/ directory away from deprecated simple_strto*() interface.

One thing to note about parse_integer() and seemingly useless casts --
range of accepted values depends on result type.

	int val;
	parse_integer(s, 0, &val);

will accept negative integers, while

	int val;
	parse_integer(s, 0, (unsigned int *)&val);

will accept only 0 and positive integers.

Cast is needed when result variable has to be of different type
for some reason.

This is very important and hopefully [knocks wood] obvious.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 mm/memcontrol.c | 19 +++++++++++--------
 mm/memtest.c    |  2 +-
 mm/page_alloc.c |  2 +-
 mm/shmem.c      | 14 ++++++++------
 4 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 14c2f20..06a920d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4096,20 +4096,23 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
 	struct fd efile;
 	struct fd cfile;
 	const char *name;
-	char *endp;
 	int ret;
 
 	buf = strstrip(buf);
 
-	efd = simple_strtoul(buf, &endp, 10);
-	if (*endp != ' ')
+	ret = parse_integer(buf, 10, &efd);
+	if (ret < 0)
+		return ret;
+	buf += ret;
+	if (*buf++ != ' ')
 		return -EINVAL;
-	buf = endp + 1;
-
-	cfd = simple_strtoul(buf, &endp, 10);
-	if ((*endp != ' ') && (*endp != '\0'))
+	ret = parse_integer(buf, 10, &efd);
+	if (ret < 0)
+		return ret;
+	buf += ret;
+	if (*buf != ' ' && *buf != '\0')
 		return -EINVAL;
-	buf = endp + 1;
+	buf++;
 
 	event = kzalloc(sizeof(*event), GFP_KERNEL);
 	if (!event)
diff --git a/mm/memtest.c b/mm/memtest.c
index 1997d93..e5e2914 100644
--- a/mm/memtest.c
+++ b/mm/memtest.c
@@ -93,7 +93,7 @@ static int memtest_pattern __initdata;
 static int __init parse_memtest(char *arg)
 {
 	if (arg)
-		memtest_pattern = simple_strtoul(arg, NULL, 0);
+		parse_integer(arg, 0, (unsigned int *)&memtest_pattern);
 	else
 		memtest_pattern = ARRAY_SIZE(patterns);
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ebffa0e..c3528a3 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6020,7 +6020,7 @@ static int __init set_hashdist(char *str)
 {
 	if (!str)
 		return 0;
-	hashdist = simple_strtoul(str, &str, 0);
+	parse_integer(str, 0, (unsigned int *)&hashdist);
 	return 1;
 }
 __setup("hashdist=", set_hashdist);
diff --git a/mm/shmem.c b/mm/shmem.c
index de98137..5ce525d 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2742,6 +2742,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
 	struct mempolicy *mpol = NULL;
 	uid_t uid;
 	gid_t gid;
+	int rv;
 
 	while (options != NULL) {
 		this_char = options;
@@ -2795,14 +2796,15 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
 		} else if (!strcmp(this_char,"mode")) {
 			if (remount)
 				continue;
-			sbinfo->mode = simple_strtoul(value, &rest, 8) & 07777;
-			if (*rest)
+			rv = parse_integer(value, 8, &sbinfo->mode);
+			if (rv < 0 || value[rv])
 				goto bad_val;
+			sbinfo->mode &= 07777;
 		} else if (!strcmp(this_char,"uid")) {
 			if (remount)
 				continue;
-			uid = simple_strtoul(value, &rest, 0);
-			if (*rest)
+			rv = parse_integer(value, 0, &uid);
+			if (rv < 0 || value[rv])
 				goto bad_val;
 			sbinfo->uid = make_kuid(current_user_ns(), uid);
 			if (!uid_valid(sbinfo->uid))
@@ -2810,8 +2812,8 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
 		} else if (!strcmp(this_char,"gid")) {
 			if (remount)
 				continue;
-			gid = simple_strtoul(value, &rest, 0);
-			if (*rest)
+			rv = parse_integer(value, 0, &gid);
+			if (rv < 0 || value[rv])
 				goto bad_val;
 			sbinfo->gid = make_kgid(current_user_ns(), gid);
 			if (!gid_valid(sbinfo->gid))
-- 
2.0.4

[PATCH 09/12] parse_integer: convert fs/

[Alexey Dobriyan]

Convert random fs/ code away from simple_strto*() interfaces.

Note about "struct simple_attr" conversion:
->set_buf is unneeded because everything can be done from stack.
->get_buf is useless as well, but that's a separate patch.
Mutex is not removed, as it may guard readers from writers,
separate story as well.
(code has been copied to arch/powerpc/.../spufs/, don't forget!)

binfmt_misc: file offset can't really be negative, type changed.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 fs/binfmt_misc.c | 12 +++++++++---
 fs/dcache.c      |  2 +-
 fs/inode.c       |  2 +-
 fs/libfs.c       | 26 ++++++++++----------------
 fs/namespace.c   |  4 ++--
 5 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 78f005f..75df426 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -47,7 +47,7 @@ enum {Enabled, Magic};
 typedef struct {
 	struct list_head list;
 	unsigned long flags;		/* type, status, etc. */
-	int offset;			/* offset of magic */
+	unsigned int offset;		/* offset of magic */
 	int size;			/* size of magic/mask */
 	char *magic;			/* magic or filename extension */
 	char *mask;			/* mask, NULL for exact match */
@@ -370,7 +370,13 @@ static Node *create_entry(const char __user *buffer, size_t count)
 		if (!s)
 			goto einval;
 		*s++ = '\0';
-		e->offset = simple_strtoul(p, &p, 10);
+		err = parse_integer(p, 10, &e->offset);
+		if (err < 0) {
+			kfree(e);
+			goto out;
+
+		}
+		p += err;
 		if (*p++)
 			goto einval;
 		pr_debug("register: offset: %#x\n", e->offset);
@@ -548,7 +554,7 @@ static void entry_status(Node *e, char *page)
 	if (!test_bit(Magic, &e->flags)) {
 		sprintf(dp, "extension .%s\n", e->magic);
 	} else {
-		dp += sprintf(dp, "offset %i\nmagic ", e->offset);
+		dp += sprintf(dp, "offset %u\nmagic ", e->offset);
 		dp = bin2hex(dp, e->magic, e->size);
 		if (e->mask) {
 			dp += sprintf(dp, "\nmask ");
diff --git a/fs/dcache.c b/fs/dcache.c
index 656ce52..b2443d9 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3374,7 +3374,7 @@ static int __init set_dhash_entries(char *str)
 {
 	if (!str)
 		return 0;
-	dhash_entries = simple_strtoul(str, &str, 0);
+	parse_integer(str, 0, &dhash_entries);
 	return 1;
 }
 __setup("dhash_entries=", set_dhash_entries);
diff --git a/fs/inode.c b/fs/inode.c
index ea37cd1..332f097 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1787,7 +1787,7 @@ static int __init set_ihash_entries(char *str)
 {
 	if (!str)
 		return 0;
-	ihash_entries = simple_strtoul(str, &str, 0);
+	parse_integer(str, 0, &ihash_entries);
 	return 1;
 }
 __setup("ihash_entries=", set_ihash_entries);
diff --git a/fs/libfs.c b/fs/libfs.c
index cb1fb4b..5b778e0 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -752,7 +752,6 @@ struct simple_attr {
 	int (*get)(void *, u64 *);
 	int (*set)(void *, u64);
 	char get_buf[24];	/* enough to store a u64 and "\n\0" */
-	char set_buf[24];
 	void *data;
 	const char *fmt;	/* format for read operation */
 	struct mutex mutex;	/* protects access to these buffers */
@@ -830,31 +829,26 @@ ssize_t simple_attr_write(struct file *file, const char __user *buf,
 			  size_t len, loff_t *ppos)
 {
 	struct simple_attr *attr;
-	u64 val;
-	size_t size;
-	ssize_t ret;
+	s64 val;
+	int ret;
 
 	attr = file->private_data;
 	if (!attr->set)
 		return -EACCES;
 
+	ret = kstrtos64_from_user(buf, len, 0, &val);
+	if (ret < 0)
+		return ret;
+
 	ret = mutex_lock_interruptible(&attr->mutex);
 	if (ret)
 		return ret;
-
-	ret = -EFAULT;
-	size = min(sizeof(attr->set_buf) - 1, len);
-	if (copy_from_user(attr->set_buf, buf, size))
-		goto out;
-
-	attr->set_buf[size] = '\0';
-	val = simple_strtoll(attr->set_buf, NULL, 0);
 	ret = attr->set(attr->data, val);
-	if (ret == 0)
-		ret = len; /* on success, claim we got the whole input */
-out:
 	mutex_unlock(&attr->mutex);
-	return ret;
+	if (ret < 0)
+		return ret;
+	/* on success, claim we got the whole input */
+	return len;
 }
 EXPORT_SYMBOL_GPL(simple_attr_write);
 
diff --git a/fs/namespace.c b/fs/namespace.c
index 1f4f9da..738144c 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -37,7 +37,7 @@ static int __init set_mhash_entries(char *str)
 {
 	if (!str)
 		return 0;
-	mhash_entries = simple_strtoul(str, &str, 0);
+	parse_integer(str, 0, &mhash_entries);
 	return 1;
 }
 __setup("mhash_entries=", set_mhash_entries);
@@ -47,7 +47,7 @@ static int __init set_mphash_entries(char *str)
 {
 	if (!str)
 		return 0;
-	mphash_entries = simple_strtoul(str, &str, 0);
+	parse_integer(str, 0, &mphash_entries);
 	return 1;
 }
 __setup("mphash_entries=", set_mphash_entries);
-- 
2.0.4

[PATCH 10/37] parse_integer: convert fs/cachefiles/

[Alexey Dobriyan]

Convert away from deprecated simple_strto*() interfaces.

Switch "unsigned long" to "unsigned int" where possible.
kstrto*() functions can't be used because of trailing "%" sign. :^)

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 fs/cachefiles/daemon.c | 84 +++++++++++++++++++++++++++-----------------------
 1 file changed, 45 insertions(+), 39 deletions(-)

diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c
index f601def..cc1a921 100644
--- a/fs/cachefiles/daemon.c
+++ b/fs/cachefiles/daemon.c
@@ -326,14 +326,15 @@ static int cachefiles_daemon_range_error(struct cachefiles_cache *cache,
  */
 static int cachefiles_daemon_frun(struct cachefiles_cache *cache, char *args)
 {
-	unsigned long frun;
+	unsigned int frun;
+	int rv;
 
 	_enter(",%s", args);
 
-	if (!*args)
-		return -EINVAL;
-
-	frun = simple_strtoul(args, &args, 10);
+	rv = parse_integer(args, 10, &frun);
+	if (rv < 0)
+		return rv;
+	args += rv;
 	if (args[0] != '%' || args[1] != '\0')
 		return -EINVAL;
 
@@ -350,14 +351,15 @@ static int cachefiles_daemon_frun(struct cachefiles_cache *cache, char *args)
  */
 static int cachefiles_daemon_fcull(struct cachefiles_cache *cache, char *args)
 {
-	unsigned long fcull;
+	unsigned int fcull;
+	int rv;
 
 	_enter(",%s", args);
 
-	if (!*args)
-		return -EINVAL;
-
-	fcull = simple_strtoul(args, &args, 10);
+	rv = parse_integer(args, 10, &fcull);
+	if (rv < 0)
+		return rv;
+	args += rv;
 	if (args[0] != '%' || args[1] != '\0')
 		return -EINVAL;
 
@@ -374,14 +376,15 @@ static int cachefiles_daemon_fcull(struct cachefiles_cache *cache, char *args)
  */
 static int cachefiles_daemon_fstop(struct cachefiles_cache *cache, char *args)
 {
-	unsigned long fstop;
+	unsigned int fstop;
+	int rv;
 
 	_enter(",%s", args);
 
-	if (!*args)
-		return -EINVAL;
-
-	fstop = simple_strtoul(args, &args, 10);
+	rv = parse_integer(args, 10, &fstop);
+	if (rv < 0)
+		return rv;
+	args += rv;
 	if (args[0] != '%' || args[1] != '\0')
 		return -EINVAL;
 
@@ -398,14 +401,15 @@ static int cachefiles_daemon_fstop(struct cachefiles_cache *cache, char *args)
  */
 static int cachefiles_daemon_brun(struct cachefiles_cache *cache, char *args)
 {
-	unsigned long brun;
+	unsigned int brun;
+	int rv;
 
 	_enter(",%s", args);
 
-	if (!*args)
-		return -EINVAL;
-
-	brun = simple_strtoul(args, &args, 10);
+	rv = parse_integer(args, 10, &brun);
+	if (rv < 0)
+		return rv;
+	args += rv;
 	if (args[0] != '%' || args[1] != '\0')
 		return -EINVAL;
 
@@ -422,14 +426,15 @@ static int cachefiles_daemon_brun(struct cachefiles_cache *cache, char *args)
  */
 static int cachefiles_daemon_bcull(struct cachefiles_cache *cache, char *args)
 {
-	unsigned long bcull;
+	unsigned int bcull;
+	int rv;
 
 	_enter(",%s", args);
 
-	if (!*args)
-		return -EINVAL;
-
-	bcull = simple_strtoul(args, &args, 10);
+	rv = parse_integer(args, 10, &bcull);
+	if (rv < 0)
+		return rv;
+	args += rv;
 	if (args[0] != '%' || args[1] != '\0')
 		return -EINVAL;
 
@@ -446,14 +451,15 @@ static int cachefiles_daemon_bcull(struct cachefiles_cache *cache, char *args)
  */
 static int cachefiles_daemon_bstop(struct cachefiles_cache *cache, char *args)
 {
-	unsigned long bstop;
+	unsigned int bstop;
+	int rv;
 
 	_enter(",%s", args);
 
-	if (!*args)
-		return -EINVAL;
-
-	bstop = simple_strtoul(args, &args, 10);
+	rv = parse_integer(args, 10, &bstop);
+	if (rv < 0)
+		return rv;
+	args += rv;
 	if (args[0] != '%' || args[1] != '\0')
 		return -EINVAL;
 
@@ -601,21 +607,21 @@ inval:
  */
 static int cachefiles_daemon_debug(struct cachefiles_cache *cache, char *args)
 {
-	unsigned long mask;
+	unsigned int mask;
+	int rv;
 
 	_enter(",%s", args);
 
-	mask = simple_strtoul(args, &args, 0);
-	if (args[0] != '\0')
-		goto inval;
-
+	rv = parse_integer(args, 0, &mask);
+	if (rv < 0)
+		return rv;
+	if (args[rv] != '\0') {
+		pr_err("debug command requires mask\n");
+		return -EINVAL;
+	}
 	cachefiles_debug = mask;
 	_leave(" = 0");
 	return 0;
-
-inval:
-	pr_err("debug command requires mask\n");
-	return -EINVAL;
 }
 
 /*
-- 
2.0.4

[PATCH 11/12] parse_integer: convert ext2, ext3, ext4

[Alexey Dobriyan]

Convert away from deprecated simple_strto*() interfaces.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 fs/ext2/super.c |  6 ++++--
 fs/ext3/super.c |  7 ++++---
 fs/ext4/super.c | 15 +++++++--------
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index d0e746e..ae9aa69 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -383,16 +383,18 @@ static unsigned long get_sb_block(void **data)
 {
 	unsigned long 	sb_block;
 	char 		*options = (char *) *data;
+	int rv;
 
 	if (!options || strncmp(options, "sb=", 3) != 0)
 		return 1;	/* Default location */
 	options += 3;
-	sb_block = simple_strtoul(options, &options, 0);
-	if (*options && *options != ',') {
+	rv = parse_integer(options, 0, &sb_block);
+	if (rv < 0 || (options[rv] && options[rv] != ',')) {
 		printk("EXT2-fs: Invalid sb specification: %s\n",
 		       (char *) *data);
 		return 1;
 	}
+	options += rv;
 	if (*options == ',')
 		options++;
 	*data = (void *) options;
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index a9312f0..d806ffe 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -902,17 +902,18 @@ static ext3_fsblk_t get_sb_block(void **data, struct super_block *sb)
 {
 	ext3_fsblk_t	sb_block;
 	char		*options = (char *) *data;
+	int rv;
 
 	if (!options || strncmp(options, "sb=", 3) != 0)
 		return 1;	/* Default location */
 	options += 3;
-	/*todo: use simple_strtoll with >32bit ext3 */
-	sb_block = simple_strtoul(options, &options, 0);
-	if (*options && *options != ',') {
+	rv = parse_integer(options, 0, &sb_block);
+	if (rv < 0 || (options[rv] && options[rv] != ',')) {
 		ext3_msg(sb, KERN_ERR, "error: invalid sb specification: %s",
 		       (char *) *data);
 		return 1;
 	}
+	options += rv;
 	if (*options == ',')
 		options++;
 	*data = (void *) options;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index f06d058..552378c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1210,18 +1210,19 @@ static ext4_fsblk_t get_sb_block(void **data)
 {
 	ext4_fsblk_t	sb_block;
 	char		*options = (char *) *data;
+	int rv;
 
 	if (!options || strncmp(options, "sb=", 3) != 0)
 		return 1;	/* Default location */
 
 	options += 3;
-	/* TODO: use simple_strtoll with >32bit ext4 */
-	sb_block = simple_strtoul(options, &options, 0);
-	if (*options && *options != ',') {
+	rv = parse_integer(options, 0, &sb_block);
+	if (rv < 0 || (options[rv] && options[rv] != ',')) {
 		printk(KERN_ERR "EXT4-fs: Invalid sb specification: %s\n",
 		       (char *) *data);
 		return 1;
 	}
+	options += rv;
 	if (*options == ',')
 		options++;
 	*data = (void *) options;
@@ -2491,10 +2492,10 @@ static ssize_t inode_readahead_blks_store(struct ext4_attr *a,
 					  struct ext4_sb_info *sbi,
 					  const char *buf, size_t count)
 {
-	unsigned long t;
+	unsigned int t;
 	int ret;
 
-	ret = kstrtoul(skip_spaces(buf), 0, &t);
+	ret = kstrtouint(skip_spaces(buf), 0, &t);
 	if (ret)
 		return ret;
 
@@ -2518,13 +2519,11 @@ static ssize_t sbi_ui_store(struct ext4_attr *a,
 			    const char *buf, size_t count)
 {
 	unsigned int *ui = (unsigned int *) (((char *) sbi) + a->u.offset);
-	unsigned long t;
 	int ret;
 
-	ret = kstrtoul(skip_spaces(buf), 0, &t);
+	ret = kstrtouint(skip_spaces(buf), 0, ui);
 	if (ret)
 		return ret;
-	*ui = t;
 	return count;
 }
 
-- 
2.0.4

[PATCH 12/12] parse_integer: convert fs/ocfs2/

[Alexey Dobriyan]

Convert away from deprecated simple_strto*() interfaces.

Autodetection of value range from type allows to remove disgusting checks:

	if ((major == LONG_MIN) || (major == LONG_MAX) ||
	    (major > (u8)-1) || (major < 1))
	        return -ERANGE;
	if ((minor == LONG_MIN) || (minor == LONG_MAX) ||
	    (minor > (u8)-1) || (minor < 0))

Poof, they're gone!

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 fs/ocfs2/cluster/heartbeat.c   | 54 ++++++++++++++++++------------------------
 fs/ocfs2/cluster/nodemanager.c | 50 +++++++++++++++++---------------------
 fs/ocfs2/stack_user.c          | 50 +++++++++++++++++++-------------------
 3 files changed, 70 insertions(+), 84 deletions(-)

diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index 16eff45..889f38c 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -1484,13 +1484,12 @@ static int o2hb_read_block_input(struct o2hb_region *reg,
 				 unsigned long *ret_bytes,
 				 unsigned int *ret_bits)
 {
-	unsigned long bytes;
-	char *p = (char *)page;
-
-	bytes = simple_strtoul(p, &p, 0);
-	if (!p || (*p && (*p != '\n')))
-		return -EINVAL;
+	unsigned int bytes;
+	int rv;
 
+	rv = kstrtouint(page, 0, &bytes);
+	if (rv < 0)
+		return rv;
 	/* Heartbeat and fs min / max block sizes are the same. */
 	if (bytes > 4096 || bytes < 512)
 		return -ERANGE;
@@ -1543,18 +1542,14 @@ static ssize_t o2hb_region_start_block_write(struct o2hb_region *reg,
 					     const char *page,
 					     size_t count)
 {
-	unsigned long long tmp;
-	char *p = (char *)page;
+	int rv;
 
 	if (reg->hr_bdev)
 		return -EINVAL;
 
-	tmp = simple_strtoull(p, &p, 0);
-	if (!p || (*p && (*p != '\n')))
-		return -EINVAL;
-
-	reg->hr_start_block = tmp;
-
+	rv = kstrtoull(page, 0, &reg->hr_start_block);
+	if (rv < 0)
+		return rv;
 	return count;
 }
 
@@ -1568,20 +1563,19 @@ static ssize_t o2hb_region_blocks_write(struct o2hb_region *reg,
 					const char *page,
 					size_t count)
 {
-	unsigned long tmp;
-	char *p = (char *)page;
+	unsigned int tmp;
+	int rv;
 
 	if (reg->hr_bdev)
 		return -EINVAL;
 
-	tmp = simple_strtoul(p, &p, 0);
-	if (!p || (*p && (*p != '\n')))
-		return -EINVAL;
-
+	rv = kstrtouint(page, 0, &tmp);
+	if (rv < 0)
+		return rv;
 	if (tmp > O2NM_MAX_NODES || tmp == 0)
 		return -ERANGE;
 
-	reg->hr_blocks = (unsigned int)tmp;
+	reg->hr_blocks = tmp;
 
 	return count;
 }
@@ -1717,9 +1711,8 @@ static ssize_t o2hb_region_dev_write(struct o2hb_region *reg,
 				     size_t count)
 {
 	struct task_struct *hb_task;
-	long fd;
+	int fd;
 	int sectsize;
-	char *p = (char *)page;
 	struct fd f;
 	struct inode *inode;
 	ssize_t ret = -EINVAL;
@@ -1733,10 +1726,9 @@ static ssize_t o2hb_region_dev_write(struct o2hb_region *reg,
 	if (o2nm_this_node() == O2NM_MAX_NODES)
 		goto out;
 
-	fd = simple_strtol(p, &p, 0);
-	if (!p || (*p && (*p != '\n')))
+	ret = kstrtoint(page, 0, &fd);
+	if (ret < 0)
 		goto out;
-
 	if (fd < 0 || fd >= INT_MAX)
 		goto out;
 
@@ -2210,12 +2202,12 @@ static ssize_t o2hb_heartbeat_group_threshold_store(struct o2hb_heartbeat_group
 						    const char *page,
 						    size_t count)
 {
-	unsigned long tmp;
-	char *p = (char *)page;
+	unsigned int tmp;
+	int rv;
 
-	tmp = simple_strtoul(p, &p, 10);
-	if (!p || (*p && (*p != '\n')))
-                return -EINVAL;
+	rv = kstrtouint(page, 10, &tmp);
+	if (rv < 0)
+		return rv;
 
 	/* this will validate ranges for us. */
 	o2hb_dead_threshold_set((unsigned int) tmp);
diff --git a/fs/ocfs2/cluster/nodemanager.c b/fs/ocfs2/cluster/nodemanager.c
index 441c84e..0381ada 100644
--- a/fs/ocfs2/cluster/nodemanager.c
+++ b/fs/ocfs2/cluster/nodemanager.c
@@ -195,13 +195,12 @@ static ssize_t o2nm_node_num_write(struct o2nm_node *node, const char *page,
 				   size_t count)
 {
 	struct o2nm_cluster *cluster = to_o2nm_cluster_from_node(node);
-	unsigned long tmp;
-	char *p = (char *)page;
-
-	tmp = simple_strtoul(p, &p, 0);
-	if (!p || (*p && (*p != '\n')))
-		return -EINVAL;
+	unsigned int tmp;
+	int rv;
 
+	rv = parse_integer(page, 0, &tmp);
+	if (rv < 0)
+		return rv;
 	if (tmp >= O2NM_MAX_NODES)
 		return -ERANGE;
 
@@ -215,16 +214,15 @@ static ssize_t o2nm_node_num_write(struct o2nm_node *node, const char *page,
 
 	write_lock(&cluster->cl_nodes_lock);
 	if (cluster->cl_nodes[tmp])
-		p = NULL;
+		rv = -EEXIST;
 	else  {
 		cluster->cl_nodes[tmp] = node;
 		node->nd_num = tmp;
 		set_bit(tmp, cluster->cl_nodes_bitmap);
 	}
 	write_unlock(&cluster->cl_nodes_lock);
-	if (p == NULL)
-		return -EEXIST;
-
+	if (rv < 0)
+		return rv;
 	return count;
 }
 static ssize_t o2nm_node_ipv4_port_read(struct o2nm_node *node, char *page)
@@ -235,13 +233,12 @@ static ssize_t o2nm_node_ipv4_port_read(struct o2nm_node *node, char *page)
 static ssize_t o2nm_node_ipv4_port_write(struct o2nm_node *node,
 					 const char *page, size_t count)
 {
-	unsigned long tmp;
-	char *p = (char *)page;
-
-	tmp = simple_strtoul(p, &p, 0);
-	if (!p || (*p && (*p != '\n')))
-		return -EINVAL;
+	u16 tmp;
+	int rv;
 
+	rv = kstrtou16(page, 0, &tmp);
+	if (rv < 0)
+		return rv;
 	if (tmp == 0)
 		return -EINVAL;
 	if (tmp >= (u16)-1)
@@ -305,13 +302,11 @@ static ssize_t o2nm_node_local_write(struct o2nm_node *node, const char *page,
 {
 	struct o2nm_cluster *cluster = to_o2nm_cluster_from_node(node);
 	unsigned long tmp;
-	char *p = (char *)page;
 	ssize_t ret;
 
-	tmp = simple_strtoul(p, &p, 0);
-	if (!p || (*p && (*p != '\n')))
-		return -EINVAL;
-
+	ret = kstrtoul(page, 0, &tmp);
+	if (ret < 0)
+		return ret;
 	tmp = !!tmp; /* boolean of whether this node wants to be local */
 
 	/* setting local turns on networking rx for now so we require having
@@ -484,16 +479,15 @@ struct o2nm_cluster_attribute {
 static ssize_t o2nm_cluster_attr_write(const char *page, ssize_t count,
                                        unsigned int *val)
 {
-	unsigned long tmp;
-	char *p = (char *)page;
-
-	tmp = simple_strtoul(p, &p, 0);
-	if (!p || (*p && (*p != '\n')))
-		return -EINVAL;
+	unsigned int tmp;
+	int rv;
 
+	rv = kstrtouint(page, 0, &tmp);
+	if (rv < 0)
+		return rv;
 	if (tmp == 0)
 		return -EINVAL;
-	if (tmp >= (u32)-1)
+	if (tmp >= (unsigned int)-1)
 		return -ERANGE;
 
 	*val = tmp;
diff --git a/fs/ocfs2/stack_user.c b/fs/ocfs2/stack_user.c
index 2768eb1..eaabdd2 100644
--- a/fs/ocfs2/stack_user.c
+++ b/fs/ocfs2/stack_user.c
@@ -368,9 +368,9 @@ static int ocfs2_control_get_this_node(void)
 static int ocfs2_control_do_setnode_msg(struct file *file,
 					struct ocfs2_control_message_setn *msg)
 {
-	long nodenum;
-	char *ptr = NULL;
 	struct ocfs2_control_private *p = file->private_data;
+	int nodenum;
+	int rv;
 
 	if (ocfs2_control_get_handshake_state(file) !=
 	    OCFS2_CONTROL_HANDSHAKE_PROTOCOL)
@@ -384,12 +384,12 @@ static int ocfs2_control_do_setnode_msg(struct file *file,
 		return -EINVAL;
 	msg->space = msg->newline = '\0';
 
-	nodenum = simple_strtol(msg->nodestr, &ptr, 16);
-	if (!ptr || *ptr)
+	rv = parse_integer(msg->nodestr, 16, &nodenum);
+	if (rv < 0)
+		return rv;
+	if (msg->nodestr[rv])
 		return -EINVAL;
-
-	if ((nodenum == LONG_MIN) || (nodenum == LONG_MAX) ||
-	    (nodenum > INT_MAX) || (nodenum < 0))
+	if (nodenum < 0)
 		return -ERANGE;
 	p->op_this_node = nodenum;
 
@@ -399,11 +399,11 @@ static int ocfs2_control_do_setnode_msg(struct file *file,
 static int ocfs2_control_do_setversion_msg(struct file *file,
 					   struct ocfs2_control_message_setv *msg)
  {
-	long major, minor;
-	char *ptr = NULL;
+	u8 major, minor;
 	struct ocfs2_control_private *p = file->private_data;
 	struct ocfs2_protocol_version *max =
 		&ocfs2_user_plugin.sp_max_proto;
+	int rv;
 
 	if (ocfs2_control_get_handshake_state(file) !=
 	    OCFS2_CONTROL_HANDSHAKE_PROTOCOL)
@@ -418,11 +418,15 @@ static int ocfs2_control_do_setversion_msg(struct file *file,
 		return -EINVAL;
 	msg->space1 = msg->space2 = msg->newline = '\0';
 
-	major = simple_strtol(msg->major, &ptr, 16);
-	if (!ptr || *ptr)
+	rv = parse_integer(msg->major, 16, &major);
+	if (rv < 0)
+		return rv;
+	if (msg->major[rv])
 		return -EINVAL;
-	minor = simple_strtol(msg->minor, &ptr, 16);
-	if (!ptr || *ptr)
+	rv = parse_integer(msg->minor, 16, &minor);
+	if (rv < 0)
+		return rv;
+	if (msg->minor[rv])
 		return -EINVAL;
 
 	/*
@@ -430,11 +434,7 @@ static int ocfs2_control_do_setversion_msg(struct file *file,
 	 * must be between 0 and 255, inclusive.  The version passed in
 	 * must be within the maximum version supported by the filesystem.
 	 */
-	if ((major == LONG_MIN) || (major == LONG_MAX) ||
-	    (major > (u8)-1) || (major < 1))
-		return -ERANGE;
-	if ((minor == LONG_MIN) || (minor == LONG_MAX) ||
-	    (minor > (u8)-1) || (minor < 0))
+	if (major < 1)
 		return -ERANGE;
 	if ((major != max->pv_major) ||
 	    (minor > max->pv_minor))
@@ -449,8 +449,8 @@ static int ocfs2_control_do_setversion_msg(struct file *file,
 static int ocfs2_control_do_down_msg(struct file *file,
 				     struct ocfs2_control_message_down *msg)
 {
-	long nodenum;
-	char *p = NULL;
+	int nodenum;
+	int rv;
 
 	if (ocfs2_control_get_handshake_state(file) !=
 	    OCFS2_CONTROL_HANDSHAKE_VALID)
@@ -465,12 +465,12 @@ static int ocfs2_control_do_down_msg(struct file *file,
 		return -EINVAL;
 	msg->space1 = msg->space2 = msg->newline = '\0';
 
-	nodenum = simple_strtol(msg->nodestr, &p, 16);
-	if (!p || *p)
+	rv = parse_integer(msg->nodestr, 16, &nodenum);
+	if (rv < 0)
+		return rv;
+	if (msg->nodestr[rv])
 		return -EINVAL;
-
-	if ((nodenum == LONG_MIN) || (nodenum == LONG_MAX) ||
-	    (nodenum > INT_MAX) || (nodenum < 0))
+	if (nodenum < 0)
 		return -ERANGE;
 
 	ocfs2_control_send_down(msg->uuid, nodenum);
-- 
2.0.4