Re: [PATCH v13 1/6] x86/boot: Introduce kstrtoull() to boot directory instead of simple_strtoull()

[Chao Fan]

On Wed, Dec 12, 2018 at 11:10:48AM +0800, Chao Fan wrote:
>Introduce kstrtoull() from lib/kstrtox.c to boot directory so that code
>in boot/ can use kstrtoull() and the old simple_strtoull() can be
>replaced.
>

Hi all,

Thanks for Boris, Baoquan and Masa's help, this PATCHSET has proceeded to
this step. With the talking in community, the key problem has been turned
from ACPI issue to kstrtoull() issue.
In this version, following the suggestion of Boris, I copy the kstrtoull()
to boot/string.c
But from last week, I was working on kstrtoull() issue in different methods
and try many times, there are several methods:
1. Copy kstrtoull() to boot/string.c
2. Include kstrtoull() to boot/string.c.
3. Use existing simple_strtoull() for now, and proceed to include kstrtoull()
   as a next work.

For method 1, PATCH is here, it's easy and clear.
For method 2, I tried and met many ifdeffery problems:
	First, the conflict of ctype.h header file.
	Second, redefine of isdigit() and memcpy() and other functions.
	Third, 'CONFIG_MODVERSIONS=y' causes ld error
	Forth, div_u64 causes make error.
Thanks to Baoquan and Masa, these problems have been fixed.
I success to make, but more warning issues is waiting to be fixed.
And I think several weeks are needed to handle the warning issue
since I am not so familiar to fix these problems.
But it blocked the main job for a long time, so if people think
the method 1 is not so good, I think method 3 is a choice.

Thanks,
Chao Fan

>Signed-off-by: Chao Fan <fanc.fnst@xxxxxxxxxxxxxx>
>---
> arch/x86/boot/string.c | 137 +++++++++++++++++++++++++++++++++++++++++
> arch/x86/boot/string.h |   2 +
> 2 files changed, 139 insertions(+)
>
>diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
>index c4428a176973..e02405f20f98 100644
>--- a/arch/x86/boot/string.c
>+++ b/arch/x86/boot/string.c
>@@ -12,7 +12,10 @@
>  * Very basic string functions
>  */
> 
>+#define _LINUX_CTYPE_H
> #include <linux/types.h>
>+#include <linux/kernel.h>
>+#include <linux/errno.h>
> #include <asm/asm.h>
> #include "ctype.h"
> #include "string.h"
>@@ -187,3 +190,137 @@ char *strchr(const char *s, int c)
> 			return NULL;
> 	return (char *)s;
> }
>+
>+static inline u64 div_u64_rem(u64 dividend, u32 divisor, u32 *remainder)
>+{
>+	union {
>+		u64 v64;
>+		u32 v32[2];
>+	} d = { dividend };
>+	u32 upper;
>+
>+	upper = d.v32[1];
>+	d.v32[1] = 0;
>+	if (upper >= divisor) {
>+		d.v32[1] = upper / divisor;
>+		upper %= divisor;
>+	}
>+	asm ("divl %2" : "=a" (d.v32[0]), "=d" (*remainder) :
>+		"rm" (divisor), "0" (d.v32[0]), "1" (upper));
>+	return d.v64;
>+}
>+
>+static inline u64 div_u64(u64 dividend, u32 divisor)
>+{
>+	u32 remainder;
>+	return div_u64_rem(dividend, divisor, &remainder);
>+}
>+
>+static inline char _tolower(const char c)
>+{
>+	return c | 0x20;
>+}
>+
>+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.
>+ * Return number of characters consumed maybe or-ed with overflow bit.
>+ * If overflow occurs, result integer (incorrect) is still returned.
>+ *
>+ * Don't you dare use this function.
>+ */
>+unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long *p)
>+{
>+	unsigned long long res;
>+	unsigned int rv;
>+
>+	res = 0;
>+	rv = 0;
>+	while (1) {
>+		unsigned int c = *s;
>+		unsigned int lc = c | 0x20; /* don't tolower() this line */
>+		unsigned int val;
>+
>+		if ('0' <= c && c <= '9')
>+			val = c - '0';
>+		else if ('a' <= lc && lc <= 'f')
>+			val = lc - 'a' + 10;
>+		else
>+			break;
>+
>+		if (val >= base)
>+			break;
>+		/*
>+		 * Check for overflow only if we are within range of
>+		 * it in the max base we support (16)
>+		 */
>+		if (unlikely(res & (~0ull << 60))) {
>+			if (res > div_u64(ULLONG_MAX - val, base))
>+				rv |= KSTRTOX_OVERFLOW;
>+		}
>+		res = res * base + val;
>+		rv++;
>+		s++;
>+	}
>+	*p = res;
>+	return rv;
>+}
>+
>+static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
>+{
>+	unsigned long long _res;
>+	unsigned int rv;
>+
>+	s = _parse_integer_fixup_radix(s, &base);
>+	rv = _parse_integer(s, base, &_res);
>+	if (rv & KSTRTOX_OVERFLOW)
>+		return -ERANGE;
>+	if (rv == 0)
>+		return -EINVAL;
>+	s += rv;
>+	if (*s == '\n')
>+		s++;
>+	if (*s)
>+		return -EINVAL;
>+	*res = _res;
>+	return 0;
>+}
>+
>+/**
>+ * kstrtoull - convert a string to an unsigned long long
>+ * @s: The start of the string. The string must be null-terminated, and may also
>+ *  include a single newline before its terminating null. The first character
>+ *  may also be a plus sign, but not a minus sign.
>+ * @base: The number base to use. The maximum supported base is 16. If base is
>+ *  given as 0, then the base of the string is automatically detected with the
>+ *  conventional semantics - If it begins with 0x the number will be parsed as a
>+ *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
>+ *  parsed as an octal number. Otherwise it will be parsed as a decimal.
>+ * @res: Where to write the result of the conversion on success.
>+ *
>+ * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
>+ * Used as a replacement for the obsolete simple_strtoull. Return code must
>+ * be checked.
>+ */
>+int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
>+{
>+	if (s[0] == '+')
>+		s++;
>+	return _kstrtoull(s, base, res);
>+}
>diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
>index 3d78e27077f4..171007c99acc 100644
>--- a/arch/x86/boot/string.h
>+++ b/arch/x86/boot/string.h
>@@ -29,4 +29,6 @@ extern unsigned int atou(const char *s);
> extern unsigned long long simple_strtoull(const char *cp, char **endp,
> 					  unsigned int base);
> 
>+int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
>+#define KSTRTOX_OVERFLOW       (1U << 31)
> #endif /* BOOT_STRING_H */
>-- 
>2.19.2
>