* [PATCH 1/2] lib: make TOLOWER macro public
@ 2011-07-18 8:23 Andy Shevchenko
2011-07-18 8:23 ` [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull() Andy Shevchenko
2011-07-18 10:25 ` [PATCH 1/2] lib: make TOLOWER macro public Alexey Dobriyan
0 siblings, 2 replies; 11+ messages in thread
From: Andy Shevchenko @ 2011-07-18 8:23 UTC (permalink / raw)
To: linux-kernel; +Cc: Andy Shevchenko, Andrew Morton, Alexey Dobriyan
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull()
2011-07-18 8:23 [PATCH 1/2] lib: make TOLOWER macro public Andy Shevchenko
@ 2011-07-18 8:23 ` Andy Shevchenko
2011-07-18 10:31 ` Alexey Dobriyan
2011-07-18 10:25 ` [PATCH 1/2] lib: make TOLOWER macro public Alexey Dobriyan
1 sibling, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2011-07-18 8:23 UTC (permalink / raw)
To: linux-kernel; +Cc: Andy Shevchenko, Andrew Morton, Alexey Dobriyan
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] lib: make TOLOWER macro public
2011-07-18 8:23 [PATCH 1/2] lib: make TOLOWER macro public Andy Shevchenko
2011-07-18 8:23 ` [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull() Andy Shevchenko
@ 2011-07-18 10:25 ` Alexey Dobriyan
2011-07-18 10:36 ` Andy Shevchenko
1 sibling, 1 reply; 11+ messages in thread
From: Alexey Dobriyan @ 2011-07-18 10:25 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-kernel, Andrew Morton
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;
> -}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull()
2011-07-18 8:23 ` [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull() Andy Shevchenko
@ 2011-07-18 10:31 ` Alexey Dobriyan
2011-07-18 10:39 ` Andy Shevchenko
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Alexey Dobriyan @ 2011-07-18 10:31 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-kernel, Andrew Morton
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] lib: make TOLOWER macro public
2011-07-18 10:25 ` [PATCH 1/2] lib: make TOLOWER macro public Alexey Dobriyan
@ 2011-07-18 10:36 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2011-07-18 10:36 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: linux-kernel, Andrew Morton
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull()
2011-07-18 10:31 ` Alexey Dobriyan
@ 2011-07-18 10:39 ` Andy Shevchenko
2011-07-18 12:57 ` [PATCHv2 1/2] lib: make _tolower() public Andy Shevchenko
2011-07-18 19:05 ` [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull() Andy Shevchenko
2 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2011-07-18 10:39 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: linux-kernel, Andrew Morton
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv2 1/2] lib: make _tolower() public
2011-07-18 10:31 ` Alexey Dobriyan
2011-07-18 10:39 ` Andy Shevchenko
@ 2011-07-18 12:57 ` Andy Shevchenko
2011-07-18 12:57 ` [PATCHv2 2/2] lib: kstrtox: add performance test case Andy Shevchenko
2011-07-18 20:52 ` [PATCHv2 1/2] lib: make _tolower() public Alexey Dobriyan
2011-07-18 19:05 ` [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull() Andy Shevchenko
2 siblings, 2 replies; 11+ messages in thread
From: Andy Shevchenko @ 2011-07-18 12:57 UTC (permalink / raw)
To: linux-kernel; +Cc: Andy Shevchenko, Andrew Morton, Alexey Dobriyan
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCHv2 2/2] lib: kstrtox: add performance test case
2011-07-18 12:57 ` [PATCHv2 1/2] lib: make _tolower() public Andy Shevchenko
@ 2011-07-18 12:57 ` Andy Shevchenko
2011-07-18 20:52 ` [PATCHv2 1/2] lib: make _tolower() public Alexey Dobriyan
1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2011-07-18 12:57 UTC (permalink / raw)
To: linux-kernel; +Cc: Andy Shevchenko, Andrew Morton, Alexey Dobriyan
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull()
2011-07-18 10:31 ` Alexey Dobriyan
2011-07-18 10:39 ` Andy Shevchenko
2011-07-18 12:57 ` [PATCHv2 1/2] lib: make _tolower() public Andy Shevchenko
@ 2011-07-18 19:05 ` Andy Shevchenko
2 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2011-07-18 19:05 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: Andy Shevchenko, linux-kernel, Andrew Morton
[-- 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¥
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv2 1/2] lib: make _tolower() public
2011-07-18 12:57 ` [PATCHv2 1/2] lib: make _tolower() public Andy Shevchenko
2011-07-18 12:57 ` [PATCHv2 2/2] lib: kstrtox: add performance test case Andy Shevchenko
@ 2011-07-18 20:52 ` Alexey Dobriyan
2011-07-19 7:17 ` [PATCHv2.1] " Andy Shevchenko
1 sibling, 1 reply; 11+ messages in thread
From: Alexey Dobriyan @ 2011-07-18 20:52 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-kernel, Andrew Morton
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;
> +}
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv2.1] lib: make _tolower() public
2011-07-18 20:52 ` [PATCHv2 1/2] lib: make _tolower() public Alexey Dobriyan
@ 2011-07-19 7:17 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2011-07-19 7:17 UTC (permalink / raw)
To: linux-kernel; +Cc: Andy Shevchenko, Andrew Morton
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-07-19 7:18 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-18 8:23 [PATCH 1/2] lib: make TOLOWER macro public Andy Shevchenko
2011-07-18 8:23 ` [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull() Andy Shevchenko
2011-07-18 10:31 ` Alexey Dobriyan
2011-07-18 10:39 ` Andy Shevchenko
2011-07-18 12:57 ` [PATCHv2 1/2] lib: make _tolower() public Andy Shevchenko
2011-07-18 12:57 ` [PATCHv2 2/2] lib: kstrtox: add performance test case Andy Shevchenko
2011-07-18 20:52 ` [PATCHv2 1/2] lib: make _tolower() public Alexey Dobriyan
2011-07-19 7:17 ` [PATCHv2.1] " Andy Shevchenko
2011-07-18 19:05 ` [PATCH 2/2] lib: call native hex_to_bin() inside _kstrtoull() Andy Shevchenko
2011-07-18 10:25 ` [PATCH 1/2] lib: make TOLOWER macro public Alexey Dobriyan
2011-07-18 10:36 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).