[PATCH 01/10] Add parse_integer() (replacement for simple_strto*())

[PATCH 01/10] 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() * 1024;
	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
  (which I personally think is stupid).

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.

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           |  203 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 310 insertions(+), 57 deletions(-)

--- 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);
--- /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) _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
--- 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
--- 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);
--- 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
--- /dev/null
+++ b/lib/parse-integer.c
@@ -0,0 +1,203 @@
+/*
+ * 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 != '\0') {
+		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)
+{
+	int rv;
+
+	if (*s == '-') {
+		return -EINVAL;
+	} else if (*s == '+') {
+		rv = __parse_integer(s + 1, base, val);
+		if (rv < 0)
+			return rv;
+		return rv + 1;
+	} else
+		return __parse_integer(s, base, val);
+}
+EXPORT_SYMBOL(_parse_integer_ull);
+
+int _parse_integer_ll(const char *s, unsigned int base, long long *val)
+{
+	unsigned long long tmp;
+	int rv;
+
+	if (*s == '-') {
+		rv = __parse_integer(s + 1, base, &tmp);
+		if (rv < 0)
+			return rv;
+		if ((long long)-tmp >= 0)
+			return -ERANGE;
+		*val = -tmp;
+		return rv + 1;
+	} else if (*s == '+') {
+		rv = __parse_integer(s + 1, base, &tmp);
+		if (rv < 0)
+			return rv;
+		if ((long long)tmp < 0)
+			return -ERANGE;
+		*val = tmp;
+		return rv + 1;
+	} else {
+		rv = __parse_integer(s, base, &tmp);
+		if (rv < 0)
+			return rv;
+		if ((long long)tmp < 0)
+			return -ERANGE;
+		*val = tmp;
+		return rv;
+	}
+}
+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);

[PATCH 02/10] 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 M.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 include/linux/kernel.h        |  124 -----------------------
 include/linux/parse-integer.h |  111 +++++++++++++++++++++
 lib/kstrtox.c                 |  222 ------------------------------------------
 lib/parse-integer.c           |   38 ++++++-
 4 files changed, 145 insertions(+), 350 deletions(-)

--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -252,130 +252,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*() or kstrto*_from_user(). */
 
 extern unsigned long simple_strtoul(const char *,char **,unsigned int);
--- a/include/linux/parse-integer.h
+++ b/include/linux/parse-integer.h
@@ -75,5 +75,116 @@ int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val
 void _parse_integer_link_time_error(void);
 
 /* internal, do not use */
+#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);
+}
+
+/* internal, do not use */
 const char *_parse_integer_fixup_radix(const char *s, unsigned int *base);
 #endif
--- 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)	\
 {									\
--- 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,27 @@ 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)
+{
+	if (base & PARSE_INTEGER_NEWLINE) {
+		unsigned long long _val;
+		int rv;
+
+		/* Accept "integer\0" or "integer\n\0" */
+		rv = ___parse_integer(s, base & ~PARSE_INTEGER_NEWLINE, &_val);
+		if (rv < 0)
+			return rv;
+		s += rv;
+		if (*s == '\n')
+			s++;
+		if (*s)
+			return -EINVAL;
+		*val = _val;
+		return 0;
+	} else
+		return ___parse_integer(s, base, val);
+}
+
 int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val)
 {
 	int rv;
@@ -73,7 +94,10 @@ int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val
 		rv = __parse_integer(s + 1, base, val);
 		if (rv < 0)
 			return rv;
-		return rv + 1;
+		if (base & PARSE_INTEGER_NEWLINE)
+			return 0;
+		else
+			return rv + 1;
 	} else
 		return __parse_integer(s, base, val);
 }
@@ -91,7 +115,10 @@ int _parse_integer_ll(const char *s, unsigned int base, long long *val)
 		if ((long long)-tmp >= 0)
 			return -ERANGE;
 		*val = -tmp;
-		return rv + 1;
+		if (base & PARSE_INTEGER_NEWLINE)
+			return 0;
+		else
+			return rv + 1;
 	} else if (*s == '+') {
 		rv = __parse_integer(s + 1, base, &tmp);
 		if (rv < 0)
@@ -99,7 +126,10 @@ int _parse_integer_ll(const char *s, unsigned int base, long long *val)
 		if ((long long)tmp < 0)
 			return -ERANGE;
 		*val = tmp;
-		return rv + 1;
+		if (base & PARSE_INTEGER_NEWLINE)
+			return 0;
+		else
+			return rv + 1;
 	} else {
 		rv = __parse_integer(s, base, &tmp);
 		if (rv < 0)
-- 
2.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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           |   38 ++++++-
 4 files changed, 143 insertions(+), 350 deletions(-)

--- 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().
--- 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
--- 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)	\
 {									\
--- 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,27 @@ 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)
+{
+	if (base & PARSE_INTEGER_NEWLINE) {
+		unsigned long long _val;
+		int rv;
+
+		/* Accept "integer\0" or "integer\n\0" */
+		rv = ___parse_integer(s, base & ~PARSE_INTEGER_NEWLINE, &_val);
+		if (rv < 0)
+			return rv;
+		s += rv;
+		if (*s == '\n')
+			s++;
+		if (*s)
+			return -EINVAL;
+		*val = _val;
+		return 0;
+	} else
+		return ___parse_integer(s, base, val);
+}
+
 int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val)
 {
 	int rv;
@@ -73,7 +94,10 @@ int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val
 		rv = __parse_integer(s + 1, base, val);
 		if (rv < 0)
 			return rv;
-		return rv + 1;
+		if (base & PARSE_INTEGER_NEWLINE)
+			return 0;
+		else
+			return rv + 1;
 	} else
 		return __parse_integer(s, base, val);
 }
@@ -91,7 +115,10 @@ int _parse_integer_ll(const char *s, unsigned int base, long long *val)
 		if ((long long)-tmp >= 0)
 			return -ERANGE;
 		*val = -tmp;
-		return rv + 1;
+		if (base & PARSE_INTEGER_NEWLINE)
+			return 0;
+		else
+			return rv + 1;
 	} else if (*s == '+') {
 		rv = __parse_integer(s + 1, base, &tmp);
 		if (rv < 0)
@@ -99,7 +126,10 @@ int _parse_integer_ll(const char *s, unsigned int base, long long *val)
 		if ((long long)tmp < 0)
 			return -ERANGE;
 		*val = tmp;
-		return rv + 1;
+		if (base & PARSE_INTEGER_NEWLINE)
+			return 0;
+		else
+			return rv + 1;
 	} else {
 		rv = __parse_integer(s, base, &tmp);
 		if (rv < 0)

[PATCH 03/10] parse_integer: convert sscanf()

[Alexey Dobriyan]

Remove base second guessing.

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(-)

--- 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;

[PATCH CORRECT 03/10] parse_integer: convert sscanf()

[Alexey Dobriyan]

Rewrite kstrto*() functions through parse_integer().

_kstrtoul() and _kstrtol() are removed because parse_integer()
can dispatch based on BITS_PER_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 M.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

	I copied patch twice, lol.

 include/linux/kernel.h        |  124 -----------------------
 include/linux/parse-integer.h |  109 ++++++++++++++++++++
 lib/kstrtox.c                 |  222 ------------------------------------------
 lib/parse-integer.c           |   38 ++++++-
 4 files changed, 143 insertions(+), 350 deletions(-)

--- 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().
--- 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
--- 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)	\
 {									\
--- 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,27 @@ 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)
+{
+	if (base & PARSE_INTEGER_NEWLINE) {
+		unsigned long long _val;
+		int rv;
+
+		/* Accept "integer\0" or "integer\n\0" */
+		rv = ___parse_integer(s, base & ~PARSE_INTEGER_NEWLINE, &_val);
+		if (rv < 0)
+			return rv;
+		s += rv;
+		if (*s == '\n')
+			s++;
+		if (*s)
+			return -EINVAL;
+		*val = _val;
+		return 0;
+	} else
+		return ___parse_integer(s, base, val);
+}
+
 int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val)
 {
 	int rv;
@@ -73,7 +94,10 @@ int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val
 		rv = __parse_integer(s + 1, base, val);
 		if (rv < 0)
 			return rv;
-		return rv + 1;
+		if (base & PARSE_INTEGER_NEWLINE)
+			return 0;
+		else
+			return rv + 1;
 	} else
 		return __parse_integer(s, base, val);
 }
@@ -91,7 +115,10 @@ int _parse_integer_ll(const char *s, unsigned int base, long long *val)
 		if ((long long)-tmp >= 0)
 			return -ERANGE;
 		*val = -tmp;
-		return rv + 1;
+		if (base & PARSE_INTEGER_NEWLINE)
+			return 0;
+		else
+			return rv + 1;
 	} else if (*s == '+') {
 		rv = __parse_integer(s + 1, base, &tmp);
 		if (rv < 0)
@@ -99,7 +126,10 @@ int _parse_integer_ll(const char *s, unsigned int base, long long *val)
 		if ((long long)tmp < 0)
 			return -ERANGE;
 		*val = tmp;
-		return rv + 1;
+		if (base & PARSE_INTEGER_NEWLINE)
+			return 0;
+		else
+			return rv + 1;
 	} else {
 		rv = __parse_integer(s, base, &tmp);
 		if (rv < 0)

[PATCH 04/10] sscanf: fix overflow

[Alexey Dobriyan]

Fun fact:

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

will return 1 (as it should), and make val=0 (as it should not).

Apart from correctness, 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(-)

--- 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);

Re: [PATCH 04/10] sscanf: fix overflow

[Rasmus Villemoes]

On Sat, May 02 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:

> Fun fact:
>
> 	uint8_t val;
> 	sscanf("256", "%hhu", &val);
>
> will return 1 (as it should), and make val=0 (as it should not).
>

What do you base these "should" and "should not" on? Both C99 and POSIX
say that the behaviour is undefined - the kernel can obviously define
its own semantics for scanf, but what do you think they should be?

If we want to correctly handle overflow, the only sane way is to make
sscanf return 0 in the above case (no conversions done). This also
seems to be what your patch does, but then I'm confused by your
first "as it should".

> Apart from correctness, 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.

Yeah, and it may be too much; sscanf("0042", "%hhu", &val") should give
42, not 4. I agree that one should be able to rely on scanf doing range
checking as part of matching.

Actually, I think one should go through all the callers of sscanf which
use a field width with an integer conversion and see if we can get rid
of it, and then rip it away from the sscanf implementation. Otherwise
there's another bug which would need fixing, namely

int x;
char rest[50];
sscanf("12345678901234567890", "%3d%s", &x, rest)

should successfully return 2 (storing 123 in x), but it can't when the
strategy is to convert as much as possible (which may then give an early
failure due to overflow), then divide by 10 until we haven't consumed
more than we're allowed.

Rasmus

Re: [PATCH 04/10] sscanf: fix overflow

[Alexey Dobriyan]

On Tue, May 5, 2015 at 12:51 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> On Sat, May 02 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
>> Fun fact:
>>
>>       uint8_t val;
>>       sscanf("256", "%hhu", &val);
>>
>> will return 1 (as it should), and make val=0 (as it should not).
>>
>
> What do you base these "should" and "should not" on? Both C99 and POSIX
> say that the behaviour is undefined - the kernel can obviously define
> its own semantics for scanf, but what do you think they should be?

POSIX can say whatever it wants, it's about common sense.

sscanf(), both kernel and libc, in this situation returns 0
when "0" character is nowhere to be found in the string!
It should either return 25 or do not return anything
because situation is ambiguous (read: set ERANGE).

> If we want to correctly handle overflow, the only sane way is to make
> sscanf return 0 in the above case (no conversions done). This also
> seems to be what your patch does, but then I'm confused by your
> first "as it should".
>
>> Apart from correctness, 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.
>
> Yeah, and it may be too much; sscanf("0042", "%hhu", &val") should give
> 42, not 4. I agree that one should be able to rely on scanf doing range
> checking as part of matching.
>
> Actually, I think one should go through all the callers of sscanf which
> use a field width with an integer conversion and see if we can get rid
> of it, and then rip it away from the sscanf implementation. Otherwise
> there's another bug which would need fixing, namely
>
> int x;
> char rest[50];
> sscanf("12345678901234567890", "%3d%s", &x, rest)
>
> should successfully return 2 (storing 123 in x), but it can't when the
> strategy is to convert as much as possible (which may then give an early
> failure due to overflow), then divide by 10 until we haven't consumed
> more than we're allowed.

Re: [PATCH 04/10] sscanf: fix overflow

[Rasmus Villemoes]

On Tue, May 05 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:

> On Tue, May 5, 2015 at 12:51 PM, Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
>> On Sat, May 02 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>>
>>> Fun fact:
>>>
>>>       uint8_t val;
>>>       sscanf("256", "%hhu", &val);
>>>
>>> will return 1 (as it should), and make val=0 (as it should not).
>>>
>>
>> What do you base these "should" and "should not" on? Both C99 and POSIX
>> say that the behaviour is undefined - the kernel can obviously define
>> its own semantics for scanf, but what do you think they should be?
>
> POSIX can say whatever it wants,

That was sort of the point, POSIX doesn't say anything, which is why I
asked what you think the semantics should be.

> it's about common sense.
>
> sscanf(), both kernel and libc, in this situation returns 0 when "0"
> character is nowhere to be found in the string!  It should either
> return 25

Really? Do you consider it common sense that sscanf("256 123", "%hhu%d", &x,
&y) can end up returning 2, putting 25 in x and 6 in y?

> or do not return anything

I agree that _that_ would be the sane thing to do, but again, I'm
confused why you then said the first example should return 1.

Rasmus

[PATCH 05/10] parse_integer: convert lib/

[Alexey Dobriyan]

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

"&(int []){0}[0]" expression is anonymous variable, don't be scared.
Filesystem option parser code does parsing 1.5 times: first to know
boundaries of a value (args[0].from, args[0].to) and then actual
parsing with match_number(). Noody knows why it does that.

match_number() needlessly allocates/duplicates memory,
parsing can be done straight from original string.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 lib/cmdline.c |   36 ++++++++++++++++++------------------
 lib/parser.c  |   29 ++++++++++++-----------------
 lib/swiotlb.c |    2 +-
 3 files changed, 31 insertions(+), 36 deletions(-)

--- 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,37 @@ 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 val;
 
-	unsigned long long ret = simple_strtoull(ptr, &endptr, 0);
-
-	switch (*endptr) {
+	ptr += parse_integer(ptr, 0, &val);
+	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;
 	}
 
 	if (retptr)
-		*retptr = endptr;
+		*retptr = (char *)ptr;
 
-	return ret;
+	return val;
 }
 EXPORT_SYMBOL(memparse);
 
--- 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;
@@ -68,19 +68,21 @@ static int match_one(char *s, const char *p, substring_t args[])
 			break;
 		}
 		case 'd':
-			simple_strtol(s, &args[argc].to, 0);
+			/* anonymous variable */
+			len = parse_integer(s, 0, &(int []){0}[0]);
 			goto num;
 		case 'u':
-			simple_strtoul(s, &args[argc].to, 0);
+			len = parse_integer(s, 0, &(unsigned int []){0}[0]);
 			goto num;
 		case 'o':
-			simple_strtoul(s, &args[argc].to, 8);
+			len = parse_integer(s, 8, &(unsigned int []){0}[0]);
 			goto num;
 		case 'x':
-			simple_strtoul(s, &args[argc].to, 16);
+			len = parse_integer(s, 16, &(unsigned int []){0}[0]);
 		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 +129,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 +139,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;
 }
 
 /**
--- 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);
 	}

Re: [PATCH 05/10] parse_integer: convert lib/

[Rasmus Villemoes]

On Sat, May 02 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:

> match_number() needlessly allocates/duplicates memory,
> parsing can be done straight from original string.

I suppose that's true, but the patch doesn't seem to do anything about
it? It's probably better to do in a separate cleanup anyway, but then
maybe this belongs as a comment below ---.


> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
>
>  lib/cmdline.c |   36 ++++++++++++++++++------------------
>  lib/parser.c  |   29 ++++++++++++-----------------
>  lib/swiotlb.c |    2 +-
>  3 files changed, 31 insertions(+), 36 deletions(-)
>
> --- 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,37 @@ 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 val;
>  
> -	unsigned long long ret = simple_strtoull(ptr, &endptr, 0);
> -
> -	switch (*endptr) {
> +	ptr += parse_integer(ptr, 0, &val);

This seems wrong. simple_strtoull used to "sanitize" the return value
from the (old) _parse_integer, so that endptr still points into the
given string. Unconditionally adding the result from parse_integer may
make ptr point far before the actual string, into who-knows-what.

> +	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;
>  	}
>  
>  	if (retptr)
> -		*retptr = endptr;
> +		*retptr = (char *)ptr;

And here we propagate that to the caller.

> -	return ret;
> +	return val;
>  }
>  EXPORT_SYMBOL(memparse);
>  
> --- 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);

Hm, I think passing cast expressions to parse_integer should be actively
discouraged. While it might work in this case, some day somebody will
copy-paste this to a place where the len variable doesn't have
sizeof==4. 

>  		else if (*p == '%') {
>  			if (*s++ != '%')
>  				return 0;
> @@ -68,19 +68,21 @@ static int match_one(char *s, const char *p, substring_t args[])
>  			break;
>  		}
>  		case 'd':
> -			simple_strtol(s, &args[argc].to, 0);
> +			/* anonymous variable */
> +			len = parse_integer(s, 0, &(int []){0}[0]);
>  			goto num;
>  		case 'u':
> -			simple_strtoul(s, &args[argc].to, 0);
> +			len = parse_integer(s, 0, &(unsigned int []){0}[0]);
>  			goto num;
>  		case 'o':
> -			simple_strtoul(s, &args[argc].to, 8);
> +			len = parse_integer(s, 8, &(unsigned int []){0}[0]);
>  			goto num;
>  		case 'x':
> -			simple_strtoul(s, &args[argc].to, 16);
> +			len = parse_integer(s, 16, &(unsigned int []){0}[0]);

I see that the commit log says "don't be scared", and the first of these
even has a comment. But is there any reason to be that clever here? I
see a couple of downsides:

* gcc has to initialize some stack memory to 0, since it cannot know it
  is only an output parameter.

* things like this usually consume an excessive amount of stack. I
  haven't been able to try your patches, but a simplified version of the
  above shows that gcc doesn't use the same stack slots for the various
  anonymous variables.

* It's unreadable, despite the comment and the commit log.

I suggest using the more obvious approach of declaring a union on the
stack and pass the address of the appropriate member.

Rasmus

Re: [PATCH 05/10] parse_integer: convert lib/

[Alexey Dobriyan]

On Mon, May 4, 2015 at 5:10 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> On Sat, May 02 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
>> match_number() needlessly allocates/duplicates memory,
>> parsing can be done straight from original string.
>
> I suppose that's true, but the patch doesn't seem to do anything about
> it? It's probably better to do in a separate cleanup anyway, but then
> maybe this belongs as a comment below ---.

Sure, it is just "raise eyebrow" moment.


>> @@ -126,38 +127,37 @@ 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 val;
>>
>> -     unsigned long long ret = simple_strtoull(ptr, &endptr, 0);
>> -
>> -     switch (*endptr) {
>> +     ptr += parse_integer(ptr, 0, &val);
>
> This seems wrong. simple_strtoull used to "sanitize" the return value
> from the (old) _parse_integer, so that endptr still points into the
> given string. Unconditionally adding the result from parse_integer may
> make ptr point far before the actual string, into who-knows-what.

When converting I tried to preserve the amount of error checking done.
simple_strtoull() either
a) return 0 and not advance pointer, or
b) return something and advance pointer.

Current code just ignores error case, so do I.

>> --- 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);
>
> Hm, I think passing cast expressions to parse_integer should be actively
> discouraged. While it might work in this case, some day somebody will
> copy-paste this to a place where the len variable doesn't have
> sizeof==4.

This cast to turn on unsigned (or signed) parsing is the only nontrivial place.

All I can say is that C programmer should be diligent about types.
Equivalent strtol() code has silent truncation. Equivalent code with any other
real function which accepts pointer has the same problem -- direct cast.

We have to live with it, I guess.

>> @@ -68,19 +68,21 @@ static int match_one(char *s, const char *p, substring_t args[])
>>                       break;
>>               }
>>               case 'd':
>> -                     simple_strtol(s, &args[argc].to, 0);
>> +                     /* anonymous variable */
>> +                     len = parse_integer(s, 0, &(int []){0}[0]);
>>                       goto num;
>>               case 'u':
>> -                     simple_strtoul(s, &args[argc].to, 0);
>> +                     len = parse_integer(s, 0, &(unsigned int []){0}[0]);
>>                       goto num;
>>               case 'o':
>> -                     simple_strtoul(s, &args[argc].to, 8);
>> +                     len = parse_integer(s, 8, &(unsigned int []){0}[0]);
>>                       goto num;
>>               case 'x':
>> -                     simple_strtoul(s, &args[argc].to, 16);
>> +                     len = parse_integer(s, 16, &(unsigned int []){0}[0]);
>
> I see that the commit log says "don't be scared", and the first of these
> even has a comment. But is there any reason to be that clever here? I
> see a couple of downsides:
>
> * gcc has to initialize some stack memory to 0, since it cannot know it
>   is only an output parameter.

Yes.

> * things like this usually consume an excessive amount of stack. I
>   haven't been able to try your patches, but a simplified version of the
>   above shows that gcc doesn't use the same stack slots for the various
>   anonymous variables.

Yes.

> * It's unreadable, despite the comment and the commit log.

> I suggest using the more obvious approach of declaring a union on the
> stack and pass the address of the appropriate member.

Union will work.

No that it matters, it's a filesystem option parsing code
which is executed rarely near the top of sys_mount(),
not exactly perfomance bottleneck.

It's a shame to lose such clever hack :-(

[PATCH 06/10] 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(-)

--- 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)
--- 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);
 
--- 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);
--- 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))

Re: [PATCH 06/10] parse_integer: convert mm/

[Rasmus Villemoes]

On Sat, May 02 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:

> 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.

... and then silently write a negative value to val if the parsed
integer happens to be larger than INT_MAX.

Again, I think passing cast expressions to parse_integer should be
verboten.

In these particular cases:

* memtest_pattern should just be unsigned int - it's only ever used as
  such anyway, and it represents a count.

* hashdist should be a boolean, but even in its current form, there's no
  reason to not just use parse_integer as-is. If people like to set it
  by passing -42 just let them.

Rasmus

Re: [PATCH 06/10] parse_integer: convert mm/

[Alexey Dobriyan]

On Mon, May 4, 2015 at 5:33 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> On Sat, May 02 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
>> 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.
>
> ... and then silently write a negative value to val if the parsed
> integer happens to be larger than INT_MAX.
>
> Again, I think passing cast expressions to parse_integer should be
> verboten.

> In these particular cases:
>
> * memtest_pattern should just be unsigned int - it's only ever used as
>   such anyway, and it represents a count.
>
> * hashdist should be a boolean, but even in its current form, there's no
>   reason to not just use parse_integer as-is. If people like to set it
>   by passing -42 just let them.

In this particular code, maybe. The real use case is this:

    int val = -1; //default
    parse_integer(s, 0, (unsigned int *)&val); //accept unsigned only

Either you add cast here, or cast where you check that value is really
unsigned after parsing:

  int val = -1;
  parse_integer(s, 0, (unsiged int *)&val);
  if (val < 0)
    return -EINVAL;

Overall number of casts is preserved.

Just grep for simple_strto* and see how much error checking people do.
The aim of the patchset is to convert code, error checking is separate story.
I fixed types and added error checks in obvious cases but really:

static int memtest_pattern __initdata;
static int __init parse_memtest(char *arg)
{
        if (arg)
                memtest_pattern = simple_strtoul(arg, NULL, 0);

"int" and strtoul() => developer doesn't care, neither do I.

[PATCH 07/10] parse_integer: convert misc fs/ code

[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(-)

--- 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 ");
--- 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);
--- 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);
--- 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);
 
--- 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);

[PATCH 08/10] fs/cachefiles/: convert to parse_integer()

[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(-)

--- 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;
 }
 
 /*

[PATCH 09/10] ocfs2: convert to parse_integer()/kstrto*()

[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(-)

--- 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);
--- 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;
--- 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);

[PATCH 10/10] ext2, ext3, ext4: convert to parse_integer()/kstrto*()

[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(-)

--- 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;
--- 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;
--- 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;
 }
 

Re: [PATCH 01/10] Add parse_integer() (replacement for simple_strto*())

[Rasmus Villemoes]

On Sat, May 02 2015, 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().
>
> 	int parse_integer(const char *s, unsigned int base, T *val);
>

I like the general idea. Few nits below (and in reply to other patches).

First: Could you tell me what tree I can commit this on top of, to see
what gcc makes of it.

> 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:

I agree that these are desirable properties, and the way it should be done.

> 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() * 1024;
> 	x += strtol();

It's nice that you list these, but...

> 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,

...agreed - I actually think it is a _good_ thing that the parse_integer
interface makes it impossible to use in these cases.

> --- 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);
> --- /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) _val = (val);	\
> +					\
> +	__builtin_choose_expr(						\
> +	__builtin_types_compatible_p(typeof(_val), signed char *),	\
> +	_parse_integer_sc(_s, _base, (void *)_val),
> \

Why the (void*) cast? Isn't _val supposed to have precisely the type
expected by _parse_integer_sc at this point?

> +	__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),			\

Ah, I see. In these cases, one probably has to do a cast to pass a
(long*) as either (int*) or (long long*) - but why not cast to the type
actually expected by _parse_integer_* instead of the launder-anything (void*)?


Another thing: It may be slightly confusing that this can't be used with
an array passed as val. This won't work:

long x[1];
rv = parse_integer(s, 0, x);

One could argue that one should pass &x[0] instead, but since this is a
macro, gcc doesn't really give a very helpful error (I just get "error:
invalid initializer"). I think it can be fixed simply by declaring _val
using typeof(&val[0]).

> +int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val)
> +{
> +	int rv;
> +
> +	if (*s == '-') {
> +		return -EINVAL;
> +	} else if (*s == '+') {
> +		rv = __parse_integer(s + 1, base, val);
> +		if (rv < 0)
> +			return rv;
> +		return rv + 1;
> +	} else
> +		return __parse_integer(s, base, val);
> +}
> +EXPORT_SYMBOL(_parse_integer_ull);
> +
> +int _parse_integer_ll(const char *s, unsigned int base, long long *val)
> +{
> +	unsigned long long tmp;
> +	int rv;
> +
> +	if (*s == '-') {
> +		rv = __parse_integer(s + 1, base, &tmp);
> +		if (rv < 0)
> +			return rv;
> +		if ((long long)-tmp >= 0)
> +			return -ERANGE;

Is there any reason to disallow "-0"?

Rasmus

Re: [PATCH 01/10] Add parse_integer() (replacement for simple_strto*())

[Alexey Dobriyan]

On Mon, May 4, 2015 at 4:24 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> On Sat, May 02 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:

>> Enter parse_integer().
>>
>>       int parse_integer(const char *s, unsigned int base, T *val);
>>
>
> I like the general idea. Few nits below (and in reply to other patches).
>
> First: Could you tell me what tree I can commit this on top of, to see
> what gcc makes of it.

Any recent kernel should be OK, code it quite self contained.
I've just applied first two patches on top 4.1-rc2.
BTW the correct order is

1) [PATCH 01/10] Add parse_integer() (replacement for simple_strto*())
*** 2) [PATCH CORRECT 03/10] parse_integer: convert sscanf()
3) [PATCH 03/10] parse_integer: convert sscanf()
4) [PATCH 04/10] sscanf: fix overflow
 ...
10) [PATCH 10/10] ext2, ext3, ext4: convert to parse_integer()/kstrto*()

I've copied patch #2 twice, so it won't apply and resent it
with subject from patch #3 to confuse everyone even more.

>> +#define parse_integer(s, base, val)  \
>> +({                                   \
>> +     const char *_s = (s);           \
>> +     unsigned int _base = (base);    \
>> +     typeof(val) _val = (val);       \
>> +                                     \
>> +     __builtin_choose_expr(                                          \
>> +     __builtin_types_compatible_p(typeof(_val), signed char *),      \
>> +     _parse_integer_sc(_s, _base, (void *)_val),
>> \
>
> Why the (void*) cast? Isn't _val supposed to have precisely the type
> expected by _parse_integer_sc at this point?
>
>> +     __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),                    \
>
> Ah, I see. In these cases, one probably has to do a cast to pass a
> (long*) as either (int*) or (long long*) - but why not cast to the type
> actually expected by _parse_integer_* instead of the launder-anything (void*)?

First macro was written without casts at all naively thinking
that gcc will only typecheck in chosen __builtin_choose_expr branch.
But it doesn't do that, remove casts and observe million of warnings.
So I shut it up with "void *". Branch is chosen base on __b_t_c_p
expression and I don't think it is possible to sneak in incorrect pointer.

> Another thing: It may be slightly confusing that this can't be used with
> an array passed as val. This won't work:
>
> long x[1];
> rv = parse_integer(s, 0, x);
> One could argue that one should pass &x[0] instead, but since this is a
> macro, gcc doesn't really give a very helpful error (I just get "error:
> invalid initializer"). I think it can be fixed simply by declaring _val
> using typeof(&val[0]).

I'd say &x[0] is way more clear that x in this case, but objection taken.
kstrto*() works in exactly same situation after all.

>> +int _parse_integer_ull(const char *s, unsigned int base, unsigned long long *val)
>> +{
>> +     int rv;
>> +
>> +     if (*s == '-') {
>> +             return -EINVAL;
>> +     } else if (*s == '+') {
>> +             rv = __parse_integer(s + 1, base, val);
>> +             if (rv < 0)
>> +                     return rv;
>> +             return rv + 1;
>> +     } else
>> +             return __parse_integer(s, base, val);
>> +}
>> +EXPORT_SYMBOL(_parse_integer_ull);
>> +
>> +int _parse_integer_ll(const char *s, unsigned int base, long long *val)
>> +{
>> +     unsigned long long tmp;
>> +     int rv;
>> +
>> +     if (*s == '-') {
>> +             rv = __parse_integer(s + 1, base, &tmp);
>> +             if (rv < 0)
>> +                     return rv;
>> +             if ((long long)-tmp >= 0)
>> +                     return -ERANGE;
>
> Is there any reason to disallow "-0"?

No! -0 is not accepted because code is copied from kstrtoll()
which doesn't accept "-0". It is even in the testsuite:

  static void __init test_kstrtoll_fail(void)
  {
  ...
/* negative zero isn't an integer in Linux */
{"-0",  0},
{"-0",  8},
{"-0",  10},
{"-0",  16},

Frankly I don't even remember why it does that, and
no one noticed until now. libc functions accept "-0".

Re: [PATCH 01/10] Add parse_integer() (replacement for simple_strto*())

[Rasmus Villemoes]

[I'm merging the subthreads below]

On Mon, May 04 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:

> On Mon, May 4, 2015 at 4:24 PM, Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
>>
>> First: Could you tell me what tree I can commit this on top of, to see
>> what gcc makes of it.
>
> Any recent kernel should be OK, code it quite self contained.
> I've just applied first two patches on top 4.1-rc2.
> BTW the correct order is
>
> 1) [PATCH 01/10] Add parse_integer() (replacement for simple_strto*())
> *** 2) [PATCH CORRECT 03/10] parse_integer: convert sscanf()
> 3) [PATCH 03/10] parse_integer: convert sscanf()
> 4) [PATCH 04/10] sscanf: fix overflow
>  ...
> 10) [PATCH 10/10] ext2, ext3, ext4: convert to parse_integer()/kstrto*()
>
> I've copied patch #2 twice, so it won't apply and resent it
> with subject from patch #3 to confuse everyone even more.

You certainly successfully confused me :-) 

>>> +#define parse_integer(s, base, val)  \
>>> +({                                   \
>>> +     const char *_s = (s);           \
>>> +     unsigned int _base = (base);    \
>>> +     typeof(val) _val = (val);       \
>>> +                                     \
>>> +     __builtin_choose_expr(                                          \
>>> +     __builtin_types_compatible_p(typeof(_val), signed char *),      \
>>> +     _parse_integer_sc(_s, _base, (void *)_val),
>>> \
>>
>> Why the (void*) cast? Isn't _val supposed to have precisely the type
>> expected by _parse_integer_sc at this point?
>>
>>> +     __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),                    \
>>
>> Ah, I see. In these cases, one probably has to do a cast to pass a
>> (long*) as either (int*) or (long long*) - but why not cast to the type
>> actually expected by _parse_integer_* instead of the launder-anything (void*)?
>
> First macro was written without casts at all naively thinking
> that gcc will only typecheck in chosen __builtin_choose_expr branch.
> But it doesn't do that, remove casts and observe million of warnings.
> So I shut it up with "void *". Branch is chosen base on __b_t_c_p
> expression and I don't think it is possible to sneak in incorrect pointer.

I see. I was worried about something like the cast laundering away a
const qualifier, but (const int*) is not _b_t_c with (int*). So ok for
now - I'll play around with this a little and see if one can get rid of
the casts anyway.
>>
>> Is there any reason to disallow "-0"?
>
> No! -0 is not accepted because code is copied from kstrtoll()
> which doesn't accept "-0". It is even in the testsuite:
>
>   static void __init test_kstrtoll_fail(void)
>   {
>   ...
> /* negative zero isn't an integer in Linux */
> {"-0",  0},
> {"-0",  8},
> {"-0",  10},
> {"-0",  16},
>
> Frankly I don't even remember why it does that, and
> no one noticed until now. libc functions accept "-0".

I think it's odd to accept "+0" but not "-0", but that's probably just
because I'm a mathematician. Am I right that you just added these test
cases because of the existing behaviour of kstrtoll? I suppose that
behaviour is just a historical accident.

If "-0" is not going to be accepted, I think that deserves a comment
(with rationale) in the parsing code and not hidden away in the test
suite.

>>>  unsigned long long memparse(const char *ptr, char **retptr)
>>>  {
>>> -     char *endptr;   /* local pointer to end of parsed string */
>>> +     unsigned long long val;
>>>
>>> -     unsigned long long ret = simple_strtoull(ptr, &endptr, 0);
>>> -
>>> -     switch (*endptr) {
>>> +     ptr += parse_integer(ptr, 0, &val);
>>
>> This seems wrong. simple_strtoull used to "sanitize" the return value
>> from the (old) _parse_integer, so that endptr still points into the
>> given string. Unconditionally adding the result from parse_integer may
>> make ptr point far before the actual string, into who-knows-what.
>
> When converting I tried to preserve the amount of error checking done.
> simple_strtoull() either
> a) return 0 and not advance pointer, or
> b) return something and advance pointer.
>

Are we talking about the same simple_strtoull? I see

	cp = _parse_integer_fixup_radix(cp, &base);
	rv = _parse_integer(cp, base, &result);
	/* FIXME */
	cp += (rv & ~KSTRTOX_OVERFLOW);

so cp is definitely advanced even in case of overflow. And in the case
of "underflow" (no digits found), the old code does initialize *result
to 0, while parse_integer by design doesn't write anything.

> Current code just ignores error case, so do I.

There's a difference between ignoring an error (which the current code
does), and ignoring _the possibility_ of an error (which the new code
does).

There are lots of callers of memparse(), and I don't think any of them
are prepared to handle *endp ending up pointing before the passed-in
string (-EINVAL == -22, -ERANGE == -34). I can easily see how that could
lead to an infinite loop, maybe worse.


[on the use of anonymous variables to get right type]

>> I suggest using the more obvious approach of declaring a union on the
>> stack and pass the address of the appropriate member.
>
> Union will work.
>
> No that it matters, it's a filesystem option parsing code
> which is executed rarely near the top of sys_mount(),
> not exactly perfomance bottleneck.
>
> It's a shame to lose such clever hack :-(

I'm sure you can find somewhere else to apply the hack :-)

> Just grep for simple_strto* and see how much error checking people do.
> The aim of the patchset is to convert code, error checking is separate
> story.

I think that's the wrong approach. One reason for adding this new
interface is precisely to facilitate error checking. So if you do some
example conversions, please do them _properly_ to show how error
checking is done. 

As you say, it's easy to find places where error checking isn't done by
just grepping for simple_strto*. I don't want to have to add
parse_integer to that list. Abusing a new interface from the beginning
isn't really a recipe for happiness.

Converting uses of simple_strto*() to parse_integer() doesn't add value
if semantics of calling code is exactly the same. And it adds negative
value if the conversions change the semantics in a surprising way, as in
the memparse case above :-(

Rasmus

Re: [PATCH 01/10] Add parse_integer() (replacement for simple_strto*())

[Alexey Dobriyan]

On Mon, May 04, 2015 at 06:44:42PM +0200, Rasmus Villemoes wrote:
> [I'm merging the subthreads below]
> 
> On Mon, May 04 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> 
> > On Mon, May 4, 2015 at 4:24 PM, Rasmus Villemoes

> >> Is there any reason to disallow "-0"?
> >
> > No! -0 is not accepted because code is copied from kstrtoll()
> > which doesn't accept "-0". It is even in the testsuite:
> >
> >   static void __init test_kstrtoll_fail(void)
> >   {
> >   ...
> > /* negative zero isn't an integer in Linux */
> > {"-0",  0},
> > {"-0",  8},
> > {"-0",  10},
> > {"-0",  16},
> >
> > Frankly I don't even remember why it does that, and
> > no one noticed until now. libc functions accept "-0".
> 
> I think it's odd to accept "+0" but not "-0", but that's probably just
> because I'm a mathematician. Am I right that you just added these test
> cases because of the existing behaviour of kstrtoll? I suppose that
> behaviour is just a historical accident.
> 
> If "-0" is not going to be accepted, I think that deserves a comment
> (with rationale) in the parsing code and not hidden away in the test
> suite.

Again, I honestly do not remember why "-0" was banned.
Let's change it to "+0 -0" for signed case, "+0" for unsigned case.

> >>>  unsigned long long memparse(const char *ptr, char **retptr)
> >>>  {
> >>> -     char *endptr;   /* local pointer to end of parsed string */
> >>> +     unsigned long long val;
> >>>
> >>> -     unsigned long long ret = simple_strtoull(ptr, &endptr, 0);
> >>> -
> >>> -     switch (*endptr) {
> >>> +     ptr += parse_integer(ptr, 0, &val);
> >>
> >> This seems wrong. simple_strtoull used to "sanitize" the return value
> >> from the (old) _parse_integer, so that endptr still points into the
> >> given string. Unconditionally adding the result from parse_integer may
> >> make ptr point far before the actual string, into who-knows-what.
> >
> > When converting I tried to preserve the amount of error checking done.
> > simple_strtoull() either
> > a) return 0 and not advance pointer, or
> > b) return something and advance pointer.
> >
> 
> Are we talking about the same simple_strtoull? I see
> 
> 	cp = _parse_integer_fixup_radix(cp, &base);
> 	rv = _parse_integer(cp, base, &result);
> 	/* FIXME */
> 	cp += (rv & ~KSTRTOX_OVERFLOW);
> 
> so cp is definitely advanced even in case of overflow. And in the case
> of "underflow" (no digits found), the old code does initialize *result
> to 0, while parse_integer by design doesn't write anything.
> 
> > Current code just ignores error case, so do I.
> 
> There's a difference between ignoring an error (which the current code
> does), and ignoring _the possibility_ of an error (which the new code
> does).
> 
> There are lots of callers of memparse(), and I don't think any of them
> are prepared to handle *endp ending up pointing before the passed-in
> string (-EINVAL == -22, -ERANGE == -34). I can easily see how that could
> lead to an infinite loop, maybe worse.

Yeah, possible bug could become worse, I'll add error checking,
but, seriously, you're defending this :^)

	case Opt_nr_inodes:
===>		/* memparse() will accept a K/M/G without a digit */
===>		if (!isdigit(*args[0].from))
===>			goto bad_val;
		pconfig->nr_inodes = memparse(args[0].from, &rest);
		break;

memparse() is misdesigned in the same sense strtoul() is misdesigned.
Every "memparse(s, NULL)" user is a bug for example.

Re: [PATCH 01/10] Add parse_integer() (replacement for simple_strto*())

[Rasmus Villemoes]

On Mon, May 04 2015, Alexey Dobriyan <adobriyan@gmail.com> wrote:

>> There are lots of callers of memparse(), and I don't think any of them
>> are prepared to handle *endp ending up pointing before the passed-in
>> string (-EINVAL == -22, -ERANGE == -34). I can easily see how that could
>> lead to an infinite loop, maybe worse.
>
> Yeah, possible bug could become worse, I'll add error checking,
> but, seriously, you're defending this :^)
>
> 	case Opt_nr_inodes:
> ===>		/* memparse() will accept a K/M/G without a digit */
> ===>		if (!isdigit(*args[0].from))
> ===>			goto bad_val;
> 		pconfig->nr_inodes = memparse(args[0].from, &rest);
> 		break;
>

No, I'm not defending memparse(), simple_strto* or any of their
callers. I'm just trying to say that you can't change the semantics of
memparse() without considering all its callers.

I don't think there's any way to "add error checking" and preserve the
exact memparse() semantic - in other words, I don't think simple_strto*
can actually be implemented in terms of parse_integer. But that's not a
bad thing - we want to get rid of those.

> memparse() is misdesigned in the same sense strtoul() is misdesigned.
> Every "memparse(s, NULL)" user is a bug for example.

Yes, memparse is misdesigned, since it doesn't have a way to indicate
error. That leads me to: There's no point in adding error checking to
the integer parsing part without also checking the left shifts for
overflow.

I think the right approach is to rename memparse to legacy_memparse and
introduce a memparse with semantics that allow error checking. One could
start by introducing that under the name sane_memparse. But there are
probably lots of simple_strto*() uses that are easier to eliminate.

Rasmus