[PATCH 1/2] lib: make TOLOWER macro public

[PATCH 1/2] lib: make TOLOWER macro public

[Andy Shevchenko]

This macro is required by *printf and kstrto* functions that are located
in the different modules. This patch makes TOLOWER macro public.
However, it's good idea to not use the macro outside of mentioned
functions.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
---
 include/linux/kernel.h |    3 +++
 lib/kstrtox.c          |   13 ++++---------
 lib/vsprintf.c         |    3 ---
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 567a6f7..8c29d3a 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -188,6 +188,9 @@ NORET_TYPE void do_exit(long error_code)
 NORET_TYPE void complete_and_exit(struct completion *, long)
 	ATTRIB_NORET;
 
+/* Works only for digits and letters, but small and fast */
+#define TOLOWER(x) ((x) | 0x20)
+
 /* 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);
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index 2dbae88..5099755 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -19,11 +19,6 @@
 #include <linux/types.h>
 #include <asm/uaccess.h>
 
-static inline char _tolower(const char c)
-{
-	return c | 0x20;
-}
-
 static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
 {
 	unsigned long long acc;
@@ -31,14 +26,14 @@ static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
 
 	if (base == 0) {
 		if (s[0] == '0') {
-			if (_tolower(s[1]) == 'x' && isxdigit(s[2]))
+			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')
+	if (base == 16 && s[0] == '0' && TOLOWER(s[1]) == 'x')
 		s += 2;
 
 	acc = 0;
@@ -48,8 +43,8 @@ static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
 
 		if ('0' <= *s && *s <= '9')
 			val = *s - '0';
-		else if ('a' <= _tolower(*s) && _tolower(*s) <= 'f')
-			val = _tolower(*s) - 'a' + 10;
+		else if ('a' <= TOLOWER(*s) && TOLOWER(*s) <= 'f')
+			val = TOLOWER(*s) - 'a' + 10;
 		else if (*s == '\n' && *(s + 1) == '\0')
 			break;
 		else
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 75bace7..29a0fe0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -31,9 +31,6 @@
 #include <asm/div64.h>
 #include <asm/sections.h>	/* for dereference_function_descriptor() */
 
-/* Works only for digits and letters, but small and fast */
-#define TOLOWER(x) ((x) | 0x20)
-
 static unsigned int simple_guess_base(const char *cp)
 {
 	if (cp[0] == '0') {
-- 
1.7.5.4

[PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull()

[Andy Shevchenko]

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
---
 lib/kstrtox.c |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index 5099755..a26d287 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -39,25 +39,18 @@ static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
 	acc = 0;
 	ok = 0;
 	while (*s) {
-		unsigned int val;
+		int val;
 
-		if ('0' <= *s && *s <= '9')
-			val = *s - '0';
-		else if ('a' <= TOLOWER(*s) && TOLOWER(*s) <= 'f')
-			val = TOLOWER(*s) - 'a' + 10;
-		else if (*s == '\n' && *(s + 1) == '\0')
+		if (unlikely(*s == '\n' && *(s + 1) == '\0'))
 			break;
-		else
-			return -EINVAL;
 
-		if (val >= base)
+		val = hex_to_bin(*s++);
+		if (val >= base || val < 0)
 			return -EINVAL;
 		if (acc > div_u64(ULLONG_MAX - val, base))
 			return -ERANGE;
 		acc = acc * base + val;
 		ok = 1;
-
-		s++;
 	}
 	if (!ok)
 		return -EINVAL;
-- 
1.7.5.4

Re: [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull()

[Alexey Dobriyan]

On Mon, Jul 18, 2011 at 11:23 AM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -39,25 +39,18 @@ static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
>        acc = 0;
>        ok = 0;
>        while (*s) {
> -               unsigned int val;
> +               int val;
>
> -               if ('0' <= *s && *s <= '9')
> -                       val = *s - '0';
> -               else if ('a' <= TOLOWER(*s) && TOLOWER(*s) <= 'f')
> -                       val = TOLOWER(*s) - 'a' + 10;
> -               else if (*s == '\n' && *(s + 1) == '\0')
> +               if (unlikely(*s == '\n' && *(s + 1) == '\0'))
>                        break;
> -               else
> -                       return -EINVAL;
>
> -               if (val >= base)
> +               val = hex_to_bin(*s++);
> +               if (val >= base || val < 0)
>                        return -EINVAL;
>                if (acc > div_u64(ULLONG_MAX - val, base))
>                        return -ERANGE;
>                acc = acc * base + val;
>                ok = 1;
> -
> -               s++;
>        }
>        if (!ok)
>                return -EINVAL;

1. unlikely() and s++ move don't have anything to do with changes
2. I don't understand desire to use some half-thought out API,
   in fact, restricting to radix 16 is arbitrary.
   Without such restriction hex_to_bin doesn't make sense.

Re: [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull()

[Andy Shevchenko]

On Mon, 2011-07-18 at 13:31 +0300, Alexey Dobriyan wrote: 
> On Mon, Jul 18, 2011 at 11:23 AM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > --- a/lib/kstrtox.c
> > +++ b/lib/kstrtox.c
> > @@ -39,25 +39,18 @@ static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
> >        acc = 0;
> >        ok = 0;
> >        while (*s) {
> > -               unsigned int val;
> > +               int val;
> >
> > -               if ('0' <= *s && *s <= '9')
> > -                       val = *s - '0';
> > -               else if ('a' <= TOLOWER(*s) && TOLOWER(*s) <= 'f')
> > -                       val = TOLOWER(*s) - 'a' + 10;
> > -               else if (*s == '\n' && *(s + 1) == '\0')
> > +               if (unlikely(*s == '\n' && *(s + 1) == '\0'))
> >                        break;
> > -               else
> > -                       return -EINVAL;
> >
> > -               if (val >= base)
> > +               val = hex_to_bin(*s++);
> > +               if (val >= base || val < 0)
> >                        return -EINVAL;
> >                if (acc > div_u64(ULLONG_MAX - val, base))
> >                        return -ERANGE;
> >                acc = acc * base + val;
> >                ok = 1;
> > -
> > -               s++;
> >        }
> >        if (!ok)
> >                return -EINVAL;
> 
> 1. unlikely() and s++ move don't have anything to do with changes
That's true. I remove those changes.

> 2. I don't understand desire to use some half-thought out API,
>    in fact, restricting to radix 16 is arbitrary.
>    Without such restriction hex_to_bin doesn't make sense.
I doubt I get it. hex_to_bin validates its input indirectly. The
behaviour of your code (without changes mentioned in 1.) is the same.

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

[PATCHv2 1/2] lib: make _tolower() public

[Andy Shevchenko]

This function is required by *printf and kstrto* functions that are located in
the different modules. This patch makes _tolower() public. However, it's good
idea to not use the helper outside of mentioned functions.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
---
 include/linux/kernel.h |    6 ++++++
 lib/kstrtox.c          |    5 -----
 lib/vsprintf.c         |   23 ++++++++++-------------
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 567a6f7..b66d937 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -189,6 +189,12 @@ NORET_TYPE void complete_and_exit(struct completion *, long)
 	ATTRIB_NORET;
 
 /* Internal, do not use. */
+static inline char _tolower(const char c)
+{
+       return c | 0x20;
+}
+
+/* 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);
 
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index 2dbae88..5e06675 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -19,11 +19,6 @@
 #include <linux/types.h>
 #include <asm/uaccess.h>
 
-static inline char _tolower(const char c)
-{
-	return c | 0x20;
-}
-
 static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
 {
 	unsigned long long acc;
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 75bace7..d7222a9 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -31,13 +31,10 @@
 #include <asm/div64.h>
 #include <asm/sections.h>	/* for dereference_function_descriptor() */
 
-/* Works only for digits and letters, but small and fast */
-#define TOLOWER(x) ((x) | 0x20)
-
 static unsigned int simple_guess_base(const char *cp)
 {
 	if (cp[0] == '0') {
-		if (TOLOWER(cp[1]) == 'x' && isxdigit(cp[2]))
+		if (_tolower(cp[1]) == 'x' && isxdigit(cp[2]))
 			return 16;
 		else
 			return 8;
@@ -59,13 +56,13 @@ unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int bas
 	if (!base)
 		base = simple_guess_base(cp);
 
-	if (base == 16 && cp[0] == '0' && TOLOWER(cp[1]) == 'x')
+	if (base == 16 && cp[0] == '0' && _tolower(cp[1]) == 'x')
 		cp += 2;
 
 	while (isxdigit(*cp)) {
 		unsigned int value;
 
-		value = isdigit(*cp) ? *cp - '0' : TOLOWER(*cp) - 'a' + 10;
+		value = isdigit(*cp) ? *cp - '0' : _tolower(*cp) - 'a' + 10;
 		if (value >= base)
 			break;
 		result = result * base + value;
@@ -1036,8 +1033,8 @@ precision:
 qualifier:
 	/* get the conversion qualifier */
 	spec->qualifier = -1;
-	if (*fmt == 'h' || TOLOWER(*fmt) == 'l' ||
-	    TOLOWER(*fmt) == 'z' || *fmt == 't') {
+	if (*fmt == 'h' || _tolower(*fmt) == 'l' ||
+	    _tolower(*fmt) == 'z' || *fmt == 't') {
 		spec->qualifier = *fmt++;
 		if (unlikely(spec->qualifier == *fmt)) {
 			if (spec->qualifier == 'l') {
@@ -1104,7 +1101,7 @@ qualifier:
 			spec->type = FORMAT_TYPE_LONG;
 		else
 			spec->type = FORMAT_TYPE_ULONG;
-	} else if (TOLOWER(spec->qualifier) == 'z') {
+	} else if (_tolower(spec->qualifier) == 'z') {
 		spec->type = FORMAT_TYPE_SIZE_T;
 	} else if (spec->qualifier == 't') {
 		spec->type = FORMAT_TYPE_PTRDIFF;
@@ -1262,7 +1259,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 			if (qualifier == 'l') {
 				long *ip = va_arg(args, long *);
 				*ip = (str - buf);
-			} else if (TOLOWER(qualifier) == 'z') {
+			} else if (_tolower(qualifier) == 'z') {
 				size_t *ip = va_arg(args, size_t *);
 				*ip = (str - buf);
 			} else {
@@ -1549,7 +1546,7 @@ do {									\
 			void *skip_arg;
 			if (qualifier == 'l')
 				skip_arg = va_arg(args, long *);
-			else if (TOLOWER(qualifier) == 'z')
+			else if (_tolower(qualifier) == 'z')
 				skip_arg = va_arg(args, size_t *);
 			else
 				skip_arg = va_arg(args, int *);
@@ -1855,8 +1852,8 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
 
 		/* get conversion qualifier */
 		qualifier = -1;
-		if (*fmt == 'h' || TOLOWER(*fmt) == 'l' ||
-		    TOLOWER(*fmt) == 'z') {
+		if (*fmt == 'h' || _tolower(*fmt) == 'l' ||
+		    _tolower(*fmt) == 'z') {
 			qualifier = *fmt++;
 			if (unlikely(qualifier == *fmt)) {
 				if (qualifier == 'h') {
-- 
1.7.5.4

[PATCHv2 2/2] lib: kstrtox: add performance test case

[Andy Shevchenko]

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
---
 lib/test-kstrtox.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/lib/test-kstrtox.c b/lib/test-kstrtox.c
index d55769d..2da5950 100644
--- a/lib/test-kstrtox.c
+++ b/lib/test-kstrtox.c
@@ -65,6 +65,12 @@ struct test_fail {
 	}								\
 }
 
+static int loop_amount;
+
+module_param(loop_amount, int, 0);
+MODULE_PARM_DESC(loop_amount,
+		 "Amount of loops for performance test (0 - no test)");
+
 static void __init test_kstrtoull_ok(void)
 {
 	DECLARE_TEST_OK(unsigned long long, struct test_ull);
@@ -707,6 +713,27 @@ static void __init test_kstrtos8_fail(void)
 	TEST_FAIL(kstrtos8, s8, "%hhd", test_s8_fail);
 }
 
+static void __init test_kstrtox_perf(void)
+{
+	struct timespec ts, ts1, ts2;
+	int count = loop_amount;
+
+	if (!loop_amount)
+		return;
+
+	getnstimeofday(&ts1);
+	while (count--) {
+		test_kstrtoull_ok();
+		test_kstrtoull_fail();
+	}
+	getnstimeofday(&ts2);
+	ts = timespec_sub(ts2, ts1);
+
+	printk(KERN_INFO "kstrox: %d conversions of strings took %lu.%09lu "
+			 "seconds\n", loop_amount,
+			 (unsigned long)ts.tv_sec, (unsigned long)ts.tv_nsec);
+}
+
 static int __init test_kstrtox_init(void)
 {
 	test_kstrtoull_ok();
@@ -733,6 +760,9 @@ static int __init test_kstrtox_init(void)
 	test_kstrtou8_fail();
 	test_kstrtos8_ok();
 	test_kstrtos8_fail();
+
+	test_kstrtox_perf();
+
 	return -EINVAL;
 }
 module_init(test_kstrtox_init);
-- 
1.7.5.4

Re: [PATCHv2 1/2] lib: make _tolower() public

[Alexey Dobriyan]

ctype.h is better place, otherwise

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

> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -189,6 +189,12 @@ NORET_TYPE void complete_and_exit(struct completion *, long)
>  	ATTRIB_NORET;
>  
>  /* Internal, do not use. */
> +static inline char _tolower(const char c)
> +{
> +       return c | 0x20;
> +}

[PATCHv2.1] lib: make _tolower() public

[Andy Shevchenko]

This function is required by *printf and kstrto* functions that are located in
the different modules. This patch makes _tolower() public. However, it's good
idea to not use the helper outside of mentioned functions.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Acked-by: Alexey Dobriyan <adobriyan@gmail.com>
---
 include/linux/ctype.h |    9 +++++++++
 lib/kstrtox.c         |    5 -----
 lib/vsprintf.c        |   23 ++++++++++-------------
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/include/linux/ctype.h b/include/linux/ctype.h
index a3d6ee0..e21e335 100644
--- a/include/linux/ctype.h
+++ b/include/linux/ctype.h
@@ -52,4 +52,13 @@ static inline unsigned char __toupper(unsigned char c)
 #define tolower(c) __tolower(c)
 #define toupper(c) __toupper(c)
 
+/*
+ * Fast implementation of tolower() for internal usage. Do not use in your
+ * code.
+ */
+static inline char _tolower(const char c)
+{
+       return c | 0x20;
+}
+
 #endif
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index 2dbae88..5e06675 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -19,11 +19,6 @@
 #include <linux/types.h>
 #include <asm/uaccess.h>
 
-static inline char _tolower(const char c)
-{
-	return c | 0x20;
-}
-
 static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
 {
 	unsigned long long acc;
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 75bace7..d7222a9 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -31,13 +31,10 @@
 #include <asm/div64.h>
 #include <asm/sections.h>	/* for dereference_function_descriptor() */
 
-/* Works only for digits and letters, but small and fast */
-#define TOLOWER(x) ((x) | 0x20)
-
 static unsigned int simple_guess_base(const char *cp)
 {
 	if (cp[0] == '0') {
-		if (TOLOWER(cp[1]) == 'x' && isxdigit(cp[2]))
+		if (_tolower(cp[1]) == 'x' && isxdigit(cp[2]))
 			return 16;
 		else
 			return 8;
@@ -59,13 +56,13 @@ unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int bas
 	if (!base)
 		base = simple_guess_base(cp);
 
-	if (base == 16 && cp[0] == '0' && TOLOWER(cp[1]) == 'x')
+	if (base == 16 && cp[0] == '0' && _tolower(cp[1]) == 'x')
 		cp += 2;
 
 	while (isxdigit(*cp)) {
 		unsigned int value;
 
-		value = isdigit(*cp) ? *cp - '0' : TOLOWER(*cp) - 'a' + 10;
+		value = isdigit(*cp) ? *cp - '0' : _tolower(*cp) - 'a' + 10;
 		if (value >= base)
 			break;
 		result = result * base + value;
@@ -1036,8 +1033,8 @@ precision:
 qualifier:
 	/* get the conversion qualifier */
 	spec->qualifier = -1;
-	if (*fmt == 'h' || TOLOWER(*fmt) == 'l' ||
-	    TOLOWER(*fmt) == 'z' || *fmt == 't') {
+	if (*fmt == 'h' || _tolower(*fmt) == 'l' ||
+	    _tolower(*fmt) == 'z' || *fmt == 't') {
 		spec->qualifier = *fmt++;
 		if (unlikely(spec->qualifier == *fmt)) {
 			if (spec->qualifier == 'l') {
@@ -1104,7 +1101,7 @@ qualifier:
 			spec->type = FORMAT_TYPE_LONG;
 		else
 			spec->type = FORMAT_TYPE_ULONG;
-	} else if (TOLOWER(spec->qualifier) == 'z') {
+	} else if (_tolower(spec->qualifier) == 'z') {
 		spec->type = FORMAT_TYPE_SIZE_T;
 	} else if (spec->qualifier == 't') {
 		spec->type = FORMAT_TYPE_PTRDIFF;
@@ -1262,7 +1259,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 			if (qualifier == 'l') {
 				long *ip = va_arg(args, long *);
 				*ip = (str - buf);
-			} else if (TOLOWER(qualifier) == 'z') {
+			} else if (_tolower(qualifier) == 'z') {
 				size_t *ip = va_arg(args, size_t *);
 				*ip = (str - buf);
 			} else {
@@ -1549,7 +1546,7 @@ do {									\
 			void *skip_arg;
 			if (qualifier == 'l')
 				skip_arg = va_arg(args, long *);
-			else if (TOLOWER(qualifier) == 'z')
+			else if (_tolower(qualifier) == 'z')
 				skip_arg = va_arg(args, size_t *);
 			else
 				skip_arg = va_arg(args, int *);
@@ -1855,8 +1852,8 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
 
 		/* get conversion qualifier */
 		qualifier = -1;
-		if (*fmt == 'h' || TOLOWER(*fmt) == 'l' ||
-		    TOLOWER(*fmt) == 'z') {
+		if (*fmt == 'h' || _tolower(*fmt) == 'l' ||
+		    _tolower(*fmt) == 'z') {
 			qualifier = *fmt++;
 			if (unlikely(qualifier == *fmt)) {
 				if (qualifier == 'h') {
-- 
1.7.5.4

Re: [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull()

[Andy Shevchenko]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 2408 bytes --]

Anyway, I wrote the small performance test case (you can see it in a
new series), which shows significant reduction of the performance
(~12%). So, just drop this patch.

On Mon, Jul 18, 2011 at 1:31 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> On Mon, Jul 18, 2011 at 11:23 AM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>> --- a/lib/kstrtox.c
>> +++ b/lib/kstrtox.c
>> @@ -39,25 +39,18 @@ static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
>>        acc = 0;
>>        ok = 0;
>>        while (*s) {
>> -               unsigned int val;
>> +               int val;
>>
>> -               if ('0' <= *s && *s <= '9')
>> -                       val = *s - '0';
>> -               else if ('a' <= TOLOWER(*s) && TOLOWER(*s) <= 'f')
>> -                       val = TOLOWER(*s) - 'a' + 10;
>> -               else if (*s == '\n' && *(s + 1) == '\0')
>> +               if (unlikely(*s == '\n' && *(s + 1) == '\0'))
>>                        break;
>> -               else
>> -                       return -EINVAL;
>>
>> -               if (val >= base)
>> +               val = hex_to_bin(*s++);
>> +               if (val >= base || val < 0)
>>                        return -EINVAL;
>>                if (acc > div_u64(ULLONG_MAX - val, base))
>>                        return -ERANGE;
>>                acc = acc * base + val;
>>                ok = 1;
>> -
>> -               s++;
>>        }
>>        if (!ok)
>>                return -EINVAL;
>
> 1. unlikely() and s++ move don't have anything to do with changes
> 2. I don't understand desire to use some half-thought out API,
>   in fact, restricting to radix 16 is arbitrary.
>   Without such restriction hex_to_bin doesn't make sense.
> --
> 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/
>



-- 
With Best Regards,
Andy Shevchenko
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 1/2] lib: make TOLOWER macro public

[Alexey Dobriyan]

NAK
This macro sucks because name doesn't reflect hackish nature.
_tolower() should be moved into public header.

On Mon, Jul 18, 2011 at 11:23 AM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> This macro is required by *printf and kstrto* functions that are located
> in the different modules. This patch makes TOLOWER macro public.
> However, it's good idea to not use the macro outside of mentioned
> functions.

> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h

> +/* Works only for digits and letters, but small and fast */
> +#define TOLOWER(x) ((x) | 0x20)

> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -19,11 +19,6 @@
>  #include <linux/types.h>
>  #include <asm/uaccess.h>
>
> -static inline char _tolower(const char c)
> -{
> -       return c | 0x20;
> -}

Re: [PATCH 1/2] lib: make TOLOWER macro public

[Andy Shevchenko]

On Mon, 2011-07-18 at 13:25 +0300, Alexey Dobriyan wrote: 
> NAK
> This macro sucks because name doesn't reflect hackish nature.
> _tolower() should be moved into public header.
> 
Makes sense. I think whole _tolower() could be moved to kernel.h.

> On Mon, Jul 18, 2011 at 11:23 AM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > This macro is required by *printf and kstrto* functions that are located
> > in the different modules. This patch makes TOLOWER macro public.
> > However, it's good idea to not use the macro outside of mentioned
> > functions.
> 
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> 
> > +/* Works only for digits and letters, but small and fast */
> > +#define TOLOWER(x) ((x) | 0x20)
> 
> > --- a/lib/kstrtox.c
> > +++ b/lib/kstrtox.c
> > @@ -19,11 +19,6 @@
> >  #include <linux/types.h>
> >  #include <asm/uaccess.h>
> >
> > -static inline char _tolower(const char c)
> > -{
> > -       return c | 0x20;
> > -}

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