* [PATCH] kstrtox: drop kstrtol()/kstrtoul() when possible
@ 2011-05-20 6:15 Alexey Dobriyan
2011-05-20 6:20 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Dobriyan @ 2011-05-20 6:15 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel, arnd, mmarek, linux-kbuild
If "long" and "long long" types are identical at runtime,
kstrtol() can be aliased to kstrtoll().
Unfortunately, one can't write
#if sizeof(long) == sizeof(long long) ...
To solve this, generate header file with sizes and alignment of
interesting types like we do with bounds.h and asm-offsets.h.
Everything above applies to "unsigned long" and kstrtoul().
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
Kbuild | 45 +++++++++++++++++++++++++++++++++++++++------
include/linux/kernel.h | 27 ++++++++-------------------
kernel/sizeof.c | 10 ++++++++++
lib/kstrtox.c | 19 +++++++++++++------
4 files changed, 70 insertions(+), 31 deletions(-)
--- a/Kbuild
+++ b/Kbuild
@@ -5,13 +5,16 @@
# 2) Generate asm-offsets.h (may need bounds.h)
# 3) Check for missing system calls
+always :=
+targets :=
+
#####
-# 1) Generate bounds.h
+# Generate bounds.h
bounds-file := include/generated/bounds.h
-always := $(bounds-file)
-targets := $(bounds-file) kernel/bounds.s
+always += $(bounds-file)
+targets += $(bounds-file) kernel/bounds.s
quiet_cmd_bounds = GEN $@
define cmd_bounds
@@ -39,8 +42,38 @@ $(obj)/$(bounds-file): kernel/bounds.s Kbuild
$(Q)mkdir -p $(dir $@)
$(call cmd,bounds)
+# generated/sizeof.h
+sizeof-file := include/generated/sizeof.h
+
+always += $(sizeof-file)
+targets += $(sizeof-file) kernel/sizeof.s
+
+quiet_cmd_sizeof = GEN $@
+define cmd_sizeof
+ ( \
+ set -e; \
+ echo "#ifndef _LINUX_SIZEOF_H"; \
+ echo "#define _LINUX_SIZEOF_H"; \
+ echo "/*"; \
+ echo " * This file is automatically generated from kernel/sizeof.c .";\
+ echo " * Modifying is futile."; \
+ echo " */"; \
+ sed -ne $(sed-y) $<; \
+ echo "#endif" \
+ ) >$@
+endef
+
+# We use internal kbuild rules to avoid the "is up to date" message from make
+kernel/sizeof.s: kernel/sizeof.c FORCE
+ $(Q)mkdir -p $(dir $@)
+ $(call if_changed_dep,cc_s_c)
+
+$(obj)/$(sizeof-file): kernel/sizeof.s Kbuild
+ $(Q)mkdir -p $(dir $@)
+ $(call cmd,sizeof)
+
#####
-# 2) Generate asm-offsets.h
+# Generate asm-offsets.h
#
offsets-file := include/generated/asm-offsets.h
@@ -85,7 +118,7 @@ $(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s Kbuild
$(call cmd,offsets)
#####
-# 3) Check for missing system calls
+# Check for missing system calls
#
quiet_cmd_syscalls = CALL $<
@@ -96,4 +129,4 @@ missing-syscalls: scripts/checksyscalls.sh FORCE
$(call cmd,syscalls)
# Keep these two files during make clean
-no-clean-files := $(bounds-file) $(offsets-file)
+no-clean-files := $(bounds-file) $(offsets-file) $(sizeof-file)
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -8,6 +8,7 @@
#define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
#ifdef __KERNEL__
+#include <generated/sizeof.h>
#include <stdarg.h>
#include <linux/linkage.h>
@@ -194,32 +195,20 @@ 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);
+#if SIZEOF_LONG == SIZEOF_LONG_LONG && ALIGNOF_LONG == ALIGNOF_LONG_LONG
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);
+ return kstrtoull(s, base, (unsigned long long *)res);
}
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);
+ return kstrtoll(s, base, (long long *)res);
}
-
+#else
+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);
+#endif
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);
new file mode 100644
--- /dev/null
+++ b/kernel/sizeof.c
@@ -0,0 +1,10 @@
+#include <linux/kbuild.h>
+
+void f(void)
+{
+ DEFINE(SIZEOF_LONG, sizeof(long));
+ DEFINE(ALIGNOF_LONG, __alignof__(long));
+
+ DEFINE(SIZEOF_LONG_LONG, sizeof(long long));
+ DEFINE(ALIGNOF_LONG_LONG, __alignof__(long long));
+}
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -11,6 +11,7 @@
*
* If -E is returned, result is not touched.
*/
+#include <generated/sizeof.h>
#include <linux/ctype.h>
#include <linux/errno.h>
#include <linux/kernel.h>
@@ -101,8 +102,14 @@ int kstrtoll(const char *s, unsigned int base, long long *res)
}
EXPORT_SYMBOL(kstrtoll);
-/* Internal, do not use. */
-int _kstrtoul(const char *s, unsigned int base, unsigned long *res)
+#if SIZEOF_LONG == SIZEOF_LONG_LONG && ALIGNOF_LONG == ALIGNOF_LONG_LONG
+/*
+ * Shortcut function call if types are indistinguishable at runtime.
+ * FYI, both __builtin_types_compatible_p(unsigned long, unsigned long long)
+ * and __builtin_types_compatible_p(long, long long) are always 0.
+ */
+#else
+int kstrtoul(const char *s, unsigned int base, unsigned long *res)
{
unsigned long long tmp;
int rv;
@@ -115,10 +122,9 @@ int _kstrtoul(const char *s, unsigned int base, unsigned long *res)
*res = tmp;
return 0;
}
-EXPORT_SYMBOL(_kstrtoul);
+EXPORT_SYMBOL(kstrtoul);
-/* Internal, do not use. */
-int _kstrtol(const char *s, unsigned int base, long *res)
+int kstrtol(const char *s, unsigned int base, long *res)
{
long long tmp;
int rv;
@@ -131,7 +137,8 @@ int _kstrtol(const char *s, unsigned int base, long *res)
*res = tmp;
return 0;
}
-EXPORT_SYMBOL(_kstrtol);
+EXPORT_SYMBOL(kstrtol);
+#endif
int kstrtouint(const char *s, unsigned int base, unsigned int *res)
{
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kstrtox: drop kstrtol()/kstrtoul() when possible
2011-05-20 6:15 [PATCH] kstrtox: drop kstrtol()/kstrtoul() when possible Alexey Dobriyan
@ 2011-05-20 6:20 ` Geert Uytterhoeven
2011-05-20 6:48 ` Alexey Dobriyan
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2011-05-20 6:20 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: akpm, linux-kernel, arnd, mmarek, linux-kbuild
On Fri, May 20, 2011 at 08:15, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> If "long" and "long long" types are identical at runtime,
> kstrtol() can be aliased to kstrtoll().
>
> Unfortunately, one can't write
>
> #if sizeof(long) == sizeof(long long) ...
One can write #ifdef CONFIG_64BIT instead.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kstrtox: drop kstrtol()/kstrtoul() when possible
2011-05-20 6:20 ` Geert Uytterhoeven
@ 2011-05-20 6:48 ` Alexey Dobriyan
2011-05-20 6:54 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Dobriyan @ 2011-05-20 6:48 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: akpm, linux-kernel, arnd, mmarek, linux-kbuild
On Fri, May 20, 2011 at 08:20:42AM +0200, Geert Uytterhoeven wrote:
> On Fri, May 20, 2011 at 08:15, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > If "long" and "long long" types are identical at runtime,
> > kstrtol() can be aliased to kstrtoll().
> >
> > Unfortunately, one can't write
> >
> > #if sizeof(long) == sizeof(long long) ...
>
> One can write #ifdef CONFIG_64BIT instead.
And alignment will match, on any arch, now and in future?
I don't think so.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kstrtox: drop kstrtol()/kstrtoul() when possible
2011-05-20 6:48 ` Alexey Dobriyan
@ 2011-05-20 6:54 ` Andrew Morton
2011-05-20 7:21 ` Alexey Dobriyan
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2011-05-20 6:54 UTC (permalink / raw)
To: Alexey Dobriyan
Cc: Geert Uytterhoeven, linux-kernel, arnd, mmarek, linux-kbuild
On Fri, 20 May 2011 09:48:27 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:
> On Fri, May 20, 2011 at 08:20:42AM +0200, Geert Uytterhoeven wrote:
> > On Fri, May 20, 2011 at 08:15, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > > If "long" and "long long" types are identical at runtime,
> > > kstrtol() can be aliased to kstrtoll().
> > >
> > > Unfortunately, one can't write
> > >
> > > __ __ __ __#if sizeof(long) == sizeof(long long) ...
> >
> > One can write #ifdef CONFIG_64BIT instead.
>
> And alignment will match, on any arch, now and in future?
> I don't think so.
Don't worry about it.
z:/usr/src/linux-2.6.39> grep -r "#[ ]*if.*CONFIG_64BIT" . | wc -l
547
So much other stuff will break that kstrtofoo is a drop in the bucket.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kstrtox: drop kstrtol()/kstrtoul() when possible
2011-05-20 6:54 ` Andrew Morton
@ 2011-05-20 7:21 ` Alexey Dobriyan
2011-05-20 7:54 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Dobriyan @ 2011-05-20 7:21 UTC (permalink / raw)
To: Andrew Morton
Cc: Geert Uytterhoeven, linux-kernel, arnd, mmarek, linux-kbuild
On Thu, May 19, 2011 at 11:54:49PM -0700, Andrew Morton wrote:
> On Fri, 20 May 2011 09:48:27 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> > On Fri, May 20, 2011 at 08:20:42AM +0200, Geert Uytterhoeven wrote:
> > > On Fri, May 20, 2011 at 08:15, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > > > If "long" and "long long" types are identical at runtime,
> > > > kstrtol() can be aliased to kstrtoll().
> > > >
> > > > Unfortunately, one can't write
> > > >
> > > > __ __ __ __#if sizeof(long) == sizeof(long long) ...
> > >
> > > One can write #ifdef CONFIG_64BIT instead.
> >
> > And alignment will match, on any arch, now and in future?
> > I don't think so.
>
> Don't worry about it.
>
> z:/usr/src/linux-2.6.39> grep -r "#[ ]*if.*CONFIG_64BIT" . | wc -l
> 547
>
> So much other stuff will break that kstrtofoo is a drop in the bucket.
Meh.
The point was that patch is obviously correct and will work in future.
CONFIG_64BIT means many things and you guys ask me to overload CONFIG_64BIT
one more time.
On X32, CONFIG_64BIT trick already doesn't technically work.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kstrtox: drop kstrtol()/kstrtoul() when possible
2011-05-20 7:21 ` Alexey Dobriyan
@ 2011-05-20 7:54 ` Geert Uytterhoeven
0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2011-05-20 7:54 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: Andrew Morton, linux-kernel, arnd, mmarek, linux-kbuild
On Fri, May 20, 2011 at 09:21, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> On Thu, May 19, 2011 at 11:54:49PM -0700, Andrew Morton wrote:
>> On Fri, 20 May 2011 09:48:27 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:
>>
>> > On Fri, May 20, 2011 at 08:20:42AM +0200, Geert Uytterhoeven wrote:
>> > > On Fri, May 20, 2011 at 08:15, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>> > > > If "long" and "long long" types are identical at runtime,
>> > > > kstrtol() can be aliased to kstrtoll().
>> > > >
>> > > > Unfortunately, one can't write
>> > > >
>> > > > __ __ __ __#if sizeof(long) == sizeof(long long) ...
>> > >
>> > > One can write #ifdef CONFIG_64BIT instead.
>> >
>> > And alignment will match, on any arch, now and in future?
>> > I don't think so.
>>
>> Don't worry about it.
>>
>> z:/usr/src/linux-2.6.39> grep -r "#[ ]*if.*CONFIG_64BIT" . | wc -l
>> 547
>>
>> So much other stuff will break that kstrtofoo is a drop in the bucket.
>
> Meh.
> The point was that patch is obviously correct and will work in future.
> CONFIG_64BIT means many things and you guys ask me to overload CONFIG_64BIT
> one more time.
In Linux, CONFIG_64BIT means that sizeof(long) == 8. Alignment of 8 byte longs
and 8 byte long long should be identical, as w.r.t. the CPU they're
the same type.
> On X32, CONFIG_64BIT trick already doesn't technically work.
Why? On X32, CONFIG_64bit is not set, and sizeof(long) == 4 and
sizeof(long long) == 8.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-05-20 7:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-20 6:15 [PATCH] kstrtox: drop kstrtol()/kstrtoul() when possible Alexey Dobriyan
2011-05-20 6:20 ` Geert Uytterhoeven
2011-05-20 6:48 ` Alexey Dobriyan
2011-05-20 6:54 ` Andrew Morton
2011-05-20 7:21 ` Alexey Dobriyan
2011-05-20 7:54 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox