Please Review :
Jaswinder Singh Rajput (6):
headers_check fix: arm, hwcap.h
headers_check fix: ia64, fpswa.h
headers_check fix: m68k, swab.h
headers_check fix: mips, ioctl.h
headers_check fix: mn10300, ptrace.h
headers_check fix: mn10300, setup.h
arch/arm/include/asm/hwcap.h | 5 +++--
arch/ia64/include/asm/fpswa.h | 3 ++-
arch/m68k/include/asm/swab.h | 2 +-
arch/mips/include/asm/ioctl.h | 4 ++++
arch/mn10300/include/asm/ptrace.h | 5 ++---
arch/mn10300/include/asm/setup.h | 3 ++-
6 files changed, 14 insertions(+), 8 deletions(-)
Thanks,
--
JSR
fix the following 'make headers_check' warning:
usr/include/asm-ia64/fpswa.h:71: extern's make no sense in userspace
Signed-off-by: Jaswinder Singh Rajput <[email protected]>
---
arch/ia64/include/asm/fpswa.h | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/ia64/include/asm/fpswa.h b/arch/ia64/include/asm/fpswa.h
index 62edfce..797b064 100644
--- a/arch/ia64/include/asm/fpswa.h
+++ b/arch/ia64/include/asm/fpswa.h
@@ -68,6 +68,7 @@ typedef struct {
efi_fpswa_t fpswa;
} fpswa_interface_t;
+#ifdef __KERNEL__
extern fpswa_interface_t *fpswa_interface;
-
+#endif
#endif /* _ASM_IA64_FPSWA_H */
--
1.6.0.6
fix the following 'make headers_check' warnings:
usr/include/asm-m68k/swab.h:4: include of <linux/types.h> is preferred over <asm/types.h>
usr/include/asm-m68k/swab.h:10: found __[us]{8,16,32,64} type without #include <linux/types.h>
Signed-off-by: Jaswinder Singh Rajput <[email protected]>
---
arch/m68k/include/asm/swab.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/m68k/include/asm/swab.h b/arch/m68k/include/asm/swab.h
index 9e3054e..5b754aa 100644
--- a/arch/m68k/include/asm/swab.h
+++ b/arch/m68k/include/asm/swab.h
@@ -1,7 +1,7 @@
#ifndef _M68K_SWAB_H
#define _M68K_SWAB_H
-#include <asm/types.h>
+#include <linux/types.h>
#include <linux/compiler.h>
#define __SWAB_64_THRU_32__
--
1.6.0.6
fix the following 'make headers_check' warning:
usr/include/asm-arm/hwcap.h:29: extern's make no sense in userspace
Signed-off-by: Jaswinder Singh Rajput <[email protected]>
---
arch/arm/include/asm/hwcap.h | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/hwcap.h b/arch/arm/include/asm/hwcap.h
index f7bd52b..48b4501 100644
--- a/arch/arm/include/asm/hwcap.h
+++ b/arch/arm/include/asm/hwcap.h
@@ -20,7 +20,8 @@
#define HWCAP_VFPv3 8192
#define HWCAP_VFPv3D16 16384
-#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
+#ifdef __KERNEL__
+#ifndef __ASSEMBLY__
/*
* This yields a mask that user programs can use to figure out what
* instruction set this cpu supports.
@@ -28,5 +29,5 @@
#define ELF_HWCAP (elf_hwcap)
extern unsigned int elf_hwcap;
#endif
-
+#endif
#endif
--
1.6.0.6
Make ioctl.h compatible with asm-generic/ioctl.h and userspace
fix the following 'make headers_check' warning:
usr/include/asm-mips/ioctl.h:64: extern's make no sense in userspace
Signed-off-by: Jaswinder Singh Rajput <[email protected]>
---
arch/mips/include/asm/ioctl.h | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/mips/include/asm/ioctl.h
b/arch/mips/include/asm/ioctl.h
index 85067e2..9161634 100644
--- a/arch/mips/include/asm/ioctl.h
+++ b/arch/mips/include/asm/ioctl.h
@@ -60,12 +60,16 @@
((nr) << _IOC_NRSHIFT) | \
((size) << _IOC_SIZESHIFT))
+#ifdef __KERNEL__
/* provoke compile error for invalid uses of size argument */
extern unsigned int __invalid_size_argument_for_IOC;
#define _IOC_TYPECHECK(t) \
((sizeof(t) == sizeof(t[1]) && \
sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
sizeof(t) : __invalid_size_argument_for_IOC)
+#else
+#define _IOC_TYPECHECK(t) (sizeof(t))
+#endif
/* used to create numbers */
#define _IO(type, nr) _IOC(_IOC_NONE, (type), (nr), 0)
--
1.6.0.6
fix the following 'make headers_check' warnings:
usr/include/asm-mn10300/setup.h:14: extern's make no sense in userspace
usr/include/asm-mn10300/setup.h:15: extern's make no sense in userspace
Signed-off-by: Jaswinder Singh Rajput <[email protected]>
---
arch/mn10300/include/asm/setup.h | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/mn10300/include/asm/setup.h b/arch/mn10300/include/asm/setup.h
index 08356c8..c229d1e 100644
--- a/arch/mn10300/include/asm/setup.h
+++ b/arch/mn10300/include/asm/setup.h
@@ -11,7 +11,8 @@
#ifndef _ASM_SETUP_H
#define _ASM_SETUP_H
+#ifdef __KERNEL__
extern void __init unit_setup(void);
extern void __init unit_init_IRQ(void);
-
+#endif
#endif /* _ASM_SETUP_H */
--
1.6.0.6
fix the following 'make headers_check' warning:
usr/include/asm-mn10300/ptrace.h:80: extern's make no sense in userspace
Signed-off-by: Jaswinder Singh Rajput <[email protected]>
---
arch/mn10300/include/asm/ptrace.h | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/mn10300/include/asm/ptrace.h b/arch/mn10300/include/asm/ptrace.h
index 7b06cc6..00aa680 100644
--- a/arch/mn10300/include/asm/ptrace.h
+++ b/arch/mn10300/include/asm/ptrace.h
@@ -77,8 +77,6 @@ struct pt_regs {
};
#endif
-extern struct pt_regs *__frame; /* current frame pointer */
-
/* Arbitrarily choose the same ptrace numbers as used by the Sparc code. */
#define PTRACE_GETREGS 12
#define PTRACE_SETREGS 13
@@ -90,6 +88,8 @@ extern struct pt_regs *__frame; /* current frame pointer */
#if defined(__KERNEL__)
+extern struct pt_regs *__frame; /* current frame pointer */
+
#if !defined(__ASSEMBLY__)
#define user_mode(regs) (((regs)->epsw & EPSW_nSL) == EPSW_nSL)
#define instruction_pointer(regs) ((regs)->pc)
@@ -99,5 +99,4 @@ extern void show_regs(struct pt_regs *);
#define profile_pc(regs) ((regs)->pc)
#endif /* __KERNEL__ */
-
#endif /* _ASM_PTRACE_H */
--
1.6.0.6
On Thu, Jun 04, 2009 at 06:05:49PM +0530, Jaswinder Singh Rajput wrote:
> Make ioctl.h compatible with asm-generic/ioctl.h and userspace
>
> fix the following 'make headers_check' warning:
>
> usr/include/asm-mips/ioctl.h:64: extern's make no sense in userspace
>
> Signed-off-by: Jaswinder Singh Rajput <[email protected]>
Thanks, applied.
Ralf
On Thu, Jun 04, 2009 at 05:57:56PM +0530, Jaswinder Singh Rajput wrote:
> fix the following 'make headers_check' warning:
I think headers_check needs fixing - there's nothing wrong with the
code as it presently stands except the tools obviously can't properly
parse C preprocessor statements.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
Hello Russell,
On Thu, 2009-06-04 at 13:53 +0100, Russell King wrote:
> On Thu, Jun 04, 2009 at 05:57:56PM +0530, Jaswinder Singh Rajput wrote:
> > fix the following 'make headers_check' warning:
>
> I think headers_check needs fixing - there's nothing wrong with the
> code as it presently stands except the tools obviously can't properly
> parse C preprocessor statements.
>
You are right.
But if we can keep things simpler why we make it complex ?
This is almost used in all header files :
#ifdef __KERNEL__
#ifndef __ASSEMBLY__
Earlier it was also used like this :
commit f884b1cf578e079f01682514ae1ae64c74586602
Author: Catalin Marinas <[email protected]>
Date: Thu Jul 12 16:10:22 2007 +0100
[ARM] 4473/2: Take the HWCAP definitions out of the elf.h file
The patch moves the HWCAP definitions and the extern elf_hwcap
declaration to the hwcap.h header file.
Signed-off-by: Catalin Marinas <[email protected]>
Signed-off-by: Russell King <[email protected]>
diff --git a/include/asm-arm/Kbuild b/include/asm-arm/Kbuild
index c68e168..73237bd 100644
--- a/include/asm-arm/Kbuild
+++ b/include/asm-arm/Kbuild
@@ -1 +1,3 @@
include include/asm-generic/Kbuild.asm
+
+unifdef-y += hwcap.h
diff --git a/include/asm-arm/elf.h b/include/asm-arm/elf.h
index 3679a8a..d7a777f 100644
--- a/include/asm-arm/elf.h
+++ b/include/asm-arm/elf.h
@@ -7,6 +7,7 @@
*/
#include <asm/ptrace.h>
#include <asm/user.h>
+#include <asm/hwcap.h>
typedef unsigned long elf_greg_t;
typedef unsigned long elf_freg_t[3];
@@ -39,31 +40,9 @@ typedef struct user_fp elf_fpregset_t;
#endif
#define ELF_ARCH EM_ARM
-/*
- * HWCAP flags - for elf_hwcap (in kernel) and AT_HWCAP
- */
-#define HWCAP_SWP 1
-#define HWCAP_HALF 2
-#define HWCAP_THUMB 4
-#define HWCAP_26BIT 8 /* Play it safe */
-#define HWCAP_FAST_MULT 16
-#define HWCAP_FPA 32
-#define HWCAP_VFP 64
-#define HWCAP_EDSP 128
-#define HWCAP_JAVA 256
-#define HWCAP_IWMMXT 512
-#define HWCAP_CRUNCH 1024
-
#ifdef __KERNEL__
#ifndef __ASSEMBLY__
/*
- * This yields a mask that user programs can use to figure out what
- * instruction set this cpu supports.
- */
-#define ELF_HWCAP (elf_hwcap)
-extern unsigned int elf_hwcap;
-
-/*
* This yields a string that ld.so will use to load implementation
* specific libraries for optimization. This is more specific in
* intent than poking at uname or /proc/cpuinfo.
diff --git a/include/asm-arm/hwcap.h b/include/asm-arm/hwcap.h
new file mode 100644
index 0000000..01a1391
--- /dev/null
+++ b/include/asm-arm/hwcap.h
@@ -0,0 +1,28 @@
+#ifndef __ASMARM_HWCAP_H
+#define __ASMARM_HWCAP_H
+
+/*
+ * HWCAP flags - for elf_hwcap (in kernel) and AT_HWCAP
+ */
+#define HWCAP_SWP 1
+#define HWCAP_HALF 2
+#define HWCAP_THUMB 4
+#define HWCAP_26BIT 8 /* Play it safe */
+#define HWCAP_FAST_MULT 16
+#define HWCAP_FPA 32
+#define HWCAP_VFP 64
+#define HWCAP_EDSP 128
+#define HWCAP_JAVA 256
+#define HWCAP_IWMMXT 512
+#define HWCAP_CRUNCH 1024
+
+#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
+/*
+ * This yields a mask that user programs can use to figure out what
+ * instruction set this cpu supports.
+ */
+#define ELF_HWCAP (elf_hwcap)
+extern unsigned int elf_hwcap;
+#endif
+
+#endif
On Thu, Jun 04, 2009 at 01:46:31PM +0100, Ralf Baechle wrote:
> On Thu, Jun 04, 2009 at 06:05:49PM +0530, Jaswinder Singh Rajput wrote:
>
> > Make ioctl.h compatible with asm-generic/ioctl.h and userspace
> >
> > fix the following 'make headers_check' warning:
> >
> > usr/include/asm-mips/ioctl.h:64: extern's make no sense in userspace
> >
> > Signed-off-by: Jaswinder Singh Rajput <[email protected]>
>
> Thanks, applied.
Hi Ralf.
Any specific reason why mips does not use include/asm-generic/ioctl.h?
Had mips done so this would not have been an issue.
Sam
On Thu, Jun 04, 2009 at 05:53:52PM +0530, Jaswinder Singh Rajput wrote:
> Please Review :
>
> Jaswinder Singh Rajput (6):
> headers_check fix: arm, hwcap.h
> headers_check fix: ia64, fpswa.h
> headers_check fix: m68k, swab.h
> headers_check fix: mips, ioctl.h
> headers_check fix: mn10300, ptrace.h
> headers_check fix: mn10300, setup.h
Please let the subject explain what was fixed.
The fact that you used headers_check to detect the
issue in the first place is less relevant.
For mips the subject could have been:
mips: use userspace compatible definition of _IOC_TYPECHECK
Did you concentrate solely on headers_check output
or did you carefully inspect the files you touched for userspace issues?
I stroingly recommend to fix all userspace issues in files touched
if this is possible.
Sam
On Thu, Jun 04, 2009 at 01:53:07PM +0100, Russell King wrote:
> On Thu, Jun 04, 2009 at 05:57:56PM +0530, Jaswinder Singh Rajput wrote:
> > fix the following 'make headers_check' warning:
>
> I think headers_check needs fixing - there's nothing wrong with the
> code as it presently stands except the tools obviously can't properly
> parse C preprocessor statements.
You are correct that headers_ceck is limited here and this patch
take some valid code and refactor it to make it headers_check compatible.
But the resulting code is as readable as before and
we keep the noise down when checking our exported headers.
Considering the effort needed to fix the tool I would request
you to take the patch anyway.
Sam
On Thu, Jun 04, 2009 at 05:59:59PM +0530, Jaswinder Singh Rajput wrote:
>
> fix the following 'make headers_check' warning:
>
> usr/include/asm-ia64/fpswa.h:71: extern's make no sense in userspace
>
> Signed-off-by: Jaswinder Singh Rajput <[email protected]>
Tony - is this file really used from userspace.
Looking at the definitions I think not.
I not then the real fix is to unexport it.
Sam
Hello Sam,
On Thu, 2009-06-04 at 22:12 +0200, Sam Ravnborg wrote:
> On Thu, Jun 04, 2009 at 05:53:52PM +0530, Jaswinder Singh Rajput wrote:
> > Please Review :
> >
> > Jaswinder Singh Rajput (6):
> > headers_check fix: arm, hwcap.h
> > headers_check fix: ia64, fpswa.h
> > headers_check fix: m68k, swab.h
> > headers_check fix: mips, ioctl.h
> > headers_check fix: mn10300, ptrace.h
> > headers_check fix: mn10300, setup.h
>
> Please let the subject explain what was fixed.
> The fact that you used headers_check to detect the
> issue in the first place is less relevant.
>
> For mips the subject could have been:
>
> mips: use userspace compatible definition of _IOC_TYPECHECK
>
OK, BTW I pointed this in commit log.
> Did you concentrate solely on headers_check output
> or did you carefully inspect the files you touched for userspace issues?
> I stroingly recommend to fix all userspace issues in files touched
> if this is possible.
>
I did my best and fix all userspace issues in above files. Now I am
waiting for the review from maintainers.
Thanks,
--
JSR
On Thursday 04 June 2009, Jaswinder Singh Rajput wrote:
> index 62edfce..797b064 100644
> --- a/arch/ia64/include/asm/fpswa.h
> +++ b/arch/ia64/include/asm/fpswa.h
> @@ -68,6 +68,7 @@ typedef struct {
> efi_fpswa_t fpswa;
> } fpswa_interface_t;
>
> +#ifdef __KERNEL__
> extern fpswa_interface_t *fpswa_interface;
> -
> +#endif
> #endif /* _ASM_IA64_FPSWA_H */
It's pretty obvious that the above data structure also doesn't
apply in user space. In fact it looks like the whole file should
just not be exported. Your patch is not helpful there because
it just silences the warning.
Arnd <><
On Thursday 04 June 2009, Sam Ravnborg wrote:
> Any specific reason why mips does not use include/asm-generic/ioctl.h?
> Had mips done so this would not have been an issue.
The original include/asm-generic/ioctl.h did not allow overriding
the values of _IOC_{SIZEBITS,DIRBITS,NONE,READ,WRITE}, so it
was initially not possible to use it.
Nowadays, you can simply use the same approach as powerpc:
#ifndef _ASM_MIPS_IOCTL_H
#define _ASM_MIPS_IOCTL_H
#define _IOC_SIZEBITS 13
#define _IOC_DIRBITS 3
#define _IOC_NONE 1U
#define _IOC_READ 2U
#define _IOC_WRITE 4U
/*
* The following are included for compatibility
*/
#define _IOC_VOID 0x20000000
#define _IOC_OUT 0x40000000
#define _IOC_IN 0x80000000
#define _IOC_INOUT (IOC_IN|IOC_OUT)
#include <asm-generic/ioctl.h>
#endif /* _ASM_MIPS_IOCTL_H */
This would indeed be a cleaner fix.
Arnd <><
On Fri, Jun 05, 2009 at 10:34:00AM +0100, Arnd Bergmann wrote:
> On Thursday 04 June 2009, Sam Ravnborg wrote:
> > Any specific reason why mips does not use include/asm-generic/ioctl.h?
> > Had mips done so this would not have been an issue.
>
> The original include/asm-generic/ioctl.h did not allow overriding
> the values of _IOC_{SIZEBITS,DIRBITS,NONE,READ,WRITE}, so it
> was initially not possible to use it.
>
> Nowadays, you can simply use the same approach as powerpc:
>
> #ifndef _ASM_MIPS_IOCTL_H
> #define _ASM_MIPS_IOCTL_H
>
> #define _IOC_SIZEBITS 13
> #define _IOC_DIRBITS 3
>
> #define _IOC_NONE 1U
> #define _IOC_READ 2U
> #define _IOC_WRITE 4U
>
> /*
> * The following are included for compatibility
> */
> #define _IOC_VOID 0x20000000
> #define _IOC_OUT 0x40000000
> #define _IOC_IN 0x80000000
> #define _IOC_INOUT (IOC_IN|IOC_OUT)
>
> #include <asm-generic/ioctl.h>
>
> #endif /* _ASM_MIPS_IOCTL_H */
>
> This would indeed be a cleaner fix.
In fact that's almost identical to what I already have. But I don't even
recall what _IOC_VOID, _IOC_OUT, _IOC_IN and _IOC_INOUT were meant to be
compatible with. They were added in 2.1.14 so presumably they've become
irrlevant, so I've dropped them. I bet nobody will notice.
Ralf
On Thu, Jun 04, 2009 at 10:16:49PM +0200, Sam Ravnborg wrote:
> On Thu, Jun 04, 2009 at 01:53:07PM +0100, Russell King wrote:
> > On Thu, Jun 04, 2009 at 05:57:56PM +0530, Jaswinder Singh Rajput wrote:
> > > fix the following 'make headers_check' warning:
> >
> > I think headers_check needs fixing - there's nothing wrong with the
> > code as it presently stands except the tools obviously can't properly
> > parse C preprocessor statements.
>
> You are correct that headers_ceck is limited here and this patch
> take some valid code and refactor it to make it headers_check compatible.
If it is just a matter of unifdef not doing the right thing, then that's
a bug in unifdef - looking at the code, it _should_ handle the case as
it's currently in the tree.
I'll spend a bit looking at this issue before applying this patch - if
the fix turns out to be trivial then we all win by not requiring people
to write stuff in unexpected (to the tools, but standard C) ways.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Fri, 5 Jun 2009, Russell King wrote:
> On Thu, Jun 04, 2009 at 10:16:49PM +0200, Sam Ravnborg wrote:
> > On Thu, Jun 04, 2009 at 01:53:07PM +0100, Russell King wrote:
> > > On Thu, Jun 04, 2009 at 05:57:56PM +0530, Jaswinder Singh Rajput wrote:
> > > > fix the following 'make headers_check' warning:
> > >
> > > I think headers_check needs fixing - there's nothing wrong with the
> > > code as it presently stands except the tools obviously can't properly
> > > parse C preprocessor statements.
> >
> > You are correct that headers_ceck is limited here and this patch
> > take some valid code and refactor it to make it headers_check compatible.
>
> If it is just a matter of unifdef not doing the right thing, then
> that's a bug in unifdef - looking at the code, it _should_ handle
> the case as it's currently in the tree.
the current version of unifdef, as it stands, has some weaknesses.
there's a newer version -- sunifdef -- that appears to be more robust
and ruthless when it comes to cleaning. just an observation.
http://www.sunifdef.strudl.org/
rday
--
========================================================================
Robert P. J. Day Waterloo, Ontario, CANADA
Linux Consulting, Training and Annoying Kernel Pedantry.
Web page: http://crashcourse.ca
Linked In: http://www.linkedin.com/in/rpjday
Twitter: http://twitter.com/rpjday
========================================================================
On Thu, Jun 04, 2009 at 10:16:49PM +0200, Sam Ravnborg wrote:
> On Thu, Jun 04, 2009 at 01:53:07PM +0100, Russell King wrote:
> > On Thu, Jun 04, 2009 at 05:57:56PM +0530, Jaswinder Singh Rajput wrote:
> > > fix the following 'make headers_check' warning:
> >
> > I think headers_check needs fixing - there's nothing wrong with the
> > code as it presently stands except the tools obviously can't properly
> > parse C preprocessor statements.
>
> You are correct that headers_ceck is limited here and this patch
> take some valid code and refactor it to make it headers_check compatible.
Okay, here's the question:
Does userspace require anything in the ifdef __ASSEMBLY__ bits?
In any case, passing -D__KERNEL__ or -U__KERNEL__ allows unifdef to
do the right thing.
The problem which unifdef has is that if it finds a symbol in an
evaluation that it doesn't know about, it fails the expansion
entirely, rather than checking whether the expansion always results
in something which should be omitted. In other words:
#if defined(__KERNEL__) && (<unknown>)
results in basically an "unknown" answer from the evaluator, where
we can see perfectly well that the expansion can never be true if
__KERNEL__ is never set.
So, the trivial answer to the problem if:
#if defined(__KERNEL__) && something-depending-on-__ASSEMBLY__
is to tell unifdef whether we want __ASSEMBLY__ defined or not defined.
This does shut up the headers_install warning from ARMs hwdef.h.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Fri, Jun 05, 2009 at 09:48:06PM +0100, Russell King wrote:
> On Thu, Jun 04, 2009 at 10:16:49PM +0200, Sam Ravnborg wrote:
> > On Thu, Jun 04, 2009 at 01:53:07PM +0100, Russell King wrote:
> > > On Thu, Jun 04, 2009 at 05:57:56PM +0530, Jaswinder Singh Rajput wrote:
> > > > fix the following 'make headers_check' warning:
> > >
> > > I think headers_check needs fixing - there's nothing wrong with the
> > > code as it presently stands except the tools obviously can't properly
> > > parse C preprocessor statements.
> >
> > You are correct that headers_ceck is limited here and this patch
> > take some valid code and refactor it to make it headers_check compatible.
>
> Okay, here's the question:
>
> Does userspace require anything in the ifdef __ASSEMBLY__ bits?
>
> In any case, passing -D__KERNEL__ or -U__KERNEL__ allows unifdef to
> do the right thing.
>
> The problem which unifdef has is that if it finds a symbol in an
> evaluation that it doesn't know about, it fails the expansion
> entirely, rather than checking whether the expansion always results
> in something which should be omitted. In other words:
>
> #if defined(__KERNEL__) && (<unknown>)
>
> results in basically an "unknown" answer from the evaluator, where
> we can see perfectly well that the expansion can never be true if
> __KERNEL__ is never set.
>
> So, the trivial answer to the problem if:
>
> #if defined(__KERNEL__) && something-depending-on-__ASSEMBLY__
>
> is to tell unifdef whether we want __ASSEMBLY__ defined or not defined.
> This does shut up the headers_install warning from ARMs hwdef.h.
That would help this onn - right.
But we have 12 other places in include/linux and 2 other places in arch/arm/include/asm
that uses this construct.
So to me this is simple a matter of:
1) add a number of specific -Dxxx to unifdef to silence those warnings
2) simplify the logic in a few places
3) fix unifdef (or use somethign better)
If I had better time I had rewritten unifdef long time ago.
And of 1) or 2) I so far has used option 2) to avoid an endless
list of config entries.
If someone uses an expression that unifdef does not cope with then
the worst things that happens is that we export a piece
of useless stuff to userspace (they never see it) and in a few cases
we spit out false warnign as in this case.
Adding only -U__ASSEMBLY__ is fine by me.
But adding all the others are not OK and here I request the simplified
logic.
Sam
>
> --
> Russell King
> Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
> maintainer of:
On Thu, 2009-06-04 at 17:53 +0530, Jaswinder Singh Rajput wrote:
> Please Review :
>
> Jaswinder Singh Rajput (6):
> headers_check fix: arm, hwcap.h
> headers_check fix: ia64, fpswa.h
> headers_check fix: m68k, swab.h
> headers_check fix: mips, ioctl.h
> headers_check fix: mn10300, ptrace.h
> headers_check fix: mn10300, setup.h
Thanks for you review, I applied all these patches to headers-check tree
for linus :
http://git.kernel.org/?p=linux/kernel/git/jaswinder/headers-check-2.6.git;a=summary
Thanks,
--
JSR
On Sat, Jun 06, 2009 at 01:40:12PM +0530, Jaswinder Singh Rajput wrote:
> On Thu, 2009-06-04 at 17:53 +0530, Jaswinder Singh Rajput wrote:
> > Please Review :
> >
> > Jaswinder Singh Rajput (6):
> > headers_check fix: arm, hwcap.h
> > headers_check fix: ia64, fpswa.h
> > headers_check fix: m68k, swab.h
> > headers_check fix: mips, ioctl.h
> > headers_check fix: mn10300, ptrace.h
> > headers_check fix: mn10300, setup.h
>
> Thanks for you review, I applied all these patches to headers-check tree
> for linus :
>
> http://git.kernel.org/?p=linux/kernel/git/jaswinder/headers-check-2.6.git;a=summary
We agreed to do mips different - right?
Sam
On Fri, 2009-06-05 at 21:48 +0100, Russell King wrote:
> On Thu, Jun 04, 2009 at 10:16:49PM +0200, Sam Ravnborg wrote:
> > On Thu, Jun 04, 2009 at 01:53:07PM +0100, Russell King wrote:
> > > On Thu, Jun 04, 2009 at 05:57:56PM +0530, Jaswinder Singh Rajput wrote:
> > > > fix the following 'make headers_check' warning:
> > >
> > > I think headers_check needs fixing - there's nothing wrong with the
> > > code as it presently stands except the tools obviously can't properly
> > > parse C preprocessor statements.
> >
> > You are correct that headers_ceck is limited here and this patch
> > take some valid code and refactor it to make it headers_check compatible.
>
> Okay, here's the question:
>
> Does userspace require anything in the ifdef __ASSEMBLY__ bits?
__ASSEMBLY__ is not specific to kernel. Any one can take benefit of it.
> In any case, passing -D__KERNEL__ or -U__KERNEL__ allows unifdef to
> do the right thing.
>
> The problem which unifdef has is that if it finds a symbol in an
> evaluation that it doesn't know about, it fails the expansion
> entirely, rather than checking whether the expansion always results
> in something which should be omitted. In other words:
>
> #if defined(__KERNEL__) && (<unknown>)
>
The problem is why you are trying to complex things which are simple and
straight.
#ifdef __KERNEL__ is for kernel
#ifndef __KERNEL__ is for userspace
Do not mix it with others defines, Whole kernel is following this
protocol why you need a exception ?
> results in basically an "unknown" answer from the evaluator, where
> we can see perfectly well that the expansion can never be true if
> __KERNEL__ is never set.
>
> So, the trivial answer to the problem if:
>
> #if defined(__KERNEL__) && something-depending-on-__ASSEMBLY__
>
> is to tell unifdef whether we want __ASSEMBLY__ defined or not defined.
> This does shut up the headers_install warning from ARMs hwdef.h.
>
#ifdef __KERNEL__
something-depending-on-__ASSEMBLY__
will not only shut up these warnings but also avoid confusion, keep
things simple and avoid unknown future problems.
My request to you please think bigger and look at bigger picture.
Thanks,
--
JSR
Hello Sam,
On Sat, 2009-06-06 at 10:43 +0200, Sam Ravnborg wrote:
> On Sat, Jun 06, 2009 at 01:40:12PM +0530, Jaswinder Singh Rajput wrote:
> > On Thu, 2009-06-04 at 17:53 +0530, Jaswinder Singh Rajput wrote:
> > > Please Review :
> > >
> > > Jaswinder Singh Rajput (6):
> > > headers_check fix: arm, hwcap.h
> > > headers_check fix: ia64, fpswa.h
> > > headers_check fix: m68k, swab.h
> > > headers_check fix: mips, ioctl.h
> > > headers_check fix: mn10300, ptrace.h
> > > headers_check fix: mn10300, setup.h
> >
> > Thanks for you review, I applied all these patches to headers-check tree
> > for linus :
> >
> > http://git.kernel.org/?p=linux/kernel/git/jaswinder/headers-check-2.6.git;a=summary
>
> We agreed to do mips different - right?
>
As I already said earlier that I mentioned compatible issue in commit
log :
"Make ioctl.h compatible with asm-generic/ioctl.h and userspace"
and it also fixes headers_check for userspace. So I kept it to maintain
harmony :
http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git&a=search&h=HEAD&st=commit&s=headers_check
Thanks,
--
JSR
On Sat, Jun 06, 2009 at 02:20:11PM +0530, Jaswinder Singh Rajput wrote:
> On Fri, 2009-06-05 at 21:48 +0100, Russell King wrote:
> > On Thu, Jun 04, 2009 at 10:16:49PM +0200, Sam Ravnborg wrote:
> > > On Thu, Jun 04, 2009 at 01:53:07PM +0100, Russell King wrote:
> > > > On Thu, Jun 04, 2009 at 05:57:56PM +0530, Jaswinder Singh Rajput wrote:
> > > > > fix the following 'make headers_check' warning:
> > > >
> > > > I think headers_check needs fixing - there's nothing wrong with the
> > > > code as it presently stands except the tools obviously can't properly
> > > > parse C preprocessor statements.
> > >
> > > You are correct that headers_ceck is limited here and this patch
> > > take some valid code and refactor it to make it headers_check compatible.
> >
> > Okay, here's the question:
> >
> > Does userspace require anything in the ifdef __ASSEMBLY__ bits?
>
> __ASSEMBLY__ is not specific to kernel. Any one can take benefit of it.
>
>
> > In any case, passing -D__KERNEL__ or -U__KERNEL__ allows unifdef to
> > do the right thing.
> >
> > The problem which unifdef has is that if it finds a symbol in an
> > evaluation that it doesn't know about, it fails the expansion
> > entirely, rather than checking whether the expansion always results
> > in something which should be omitted. In other words:
> >
> > #if defined(__KERNEL__) && (<unknown>)
> >
>
> The problem is why you are trying to complex things which are simple and
> straight.
What you're saying is "you may not follow valid C ways of expressing
conditional compilation" to which I say "go and piss in someone elses
pool".
Since the fix for this in unifdef is soo trivial, and it doesn't require
people to write stuff in ways that stupid idiotic tools can understand,
I'm NEVER going to apply the fix to hwdef.h.
So you now have two options: either supply the (correct) additional
parameter to unifdef, or ignore the stupid idiotic warning message.
> My request to you please think bigger and look at bigger picture.
Same response to you actually. What I'm doing is valid C and there
has NEVER been this convention you're trying to make out.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Sat, 2009-06-06 at 10:12 +0100, Russell King wrote:
> On Sat, Jun 06, 2009 at 02:20:11PM +0530, Jaswinder Singh Rajput wrote:
> >
> > The problem is why you are trying to complex things which are simple and
> > straight.
>
> What you're saying is "you may not follow valid C ways of expressing
> conditional compilation" to which I say "go and piss in someone elses
> pool".
>
Hmm, again you are trying to complex things.
Issue is not of valid C or not.
Issue is #if defined(__KERNEL__) && (<unknown>) is WRONG.
It should be :
#ifdef __KERNEL__
#ifdef <unknown>
otherwise how you will pass it to userspace.
Why you are looking on kernel side only why do not you think about user
space ?
please check how it will look at usr/include/asm-arm/hwcap.h
> Since the fix for this in unifdef is soo trivial, and it doesn't require
> people to write stuff in ways that stupid idiotic tools can understand,
> I'm NEVER going to apply the fix to hwdef.h.
>
> So you now have two options: either supply the (correct) additional
> parameter to unifdef, or ignore the stupid idiotic warning message.
>
I will go with 3rd option.
Thanks,
--
JSR
Linus,
Please pull headers_check fixes:
The following changes since commit 064e38aaded5269e573ac1c765284fd65c8ebd13:
Linus Torvalds (1):
Merge git://git.kernel.org/.../mason/btrfs-unstable
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/headers-check-2.6.git master
Jaswinder Singh Rajput (6):
headers_check fix: arm, hwcap.h
headers_check fix: ia64, fpswa.h
headers_check fix: m68k, swab.h
headers_check fix: mips, ioctl.h
headers_check fix: mn10300, ptrace.h
headers_check fix: mn10300, setup.h
arch/arm/include/asm/hwcap.h | 5 +++--
arch/ia64/include/asm/fpswa.h | 3 ++-
arch/m68k/include/asm/swab.h | 2 +-
arch/mips/include/asm/ioctl.h | 4 ++++
arch/mn10300/include/asm/ptrace.h | 5 ++---
arch/mn10300/include/asm/setup.h | 3 ++-
6 files changed, 14 insertions(+), 8 deletions(-)
Complete diff:
diff --git a/arch/arm/include/asm/hwcap.h b/arch/arm/include/asm/hwcap.h
index f7bd52b..48b4501 100644
--- a/arch/arm/include/asm/hwcap.h
+++ b/arch/arm/include/asm/hwcap.h
@@ -20,7 +20,8 @@
#define HWCAP_VFPv3 8192
#define HWCAP_VFPv3D16 16384
-#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
+#ifdef __KERNEL__
+#ifndef __ASSEMBLY__
/*
* This yields a mask that user programs can use to figure out what
* instruction set this cpu supports.
@@ -28,5 +29,5 @@
#define ELF_HWCAP (elf_hwcap)
extern unsigned int elf_hwcap;
#endif
-
+#endif
#endif
diff --git a/arch/ia64/include/asm/fpswa.h b/arch/ia64/include/asm/fpswa.h
index 62edfce..797b064 100644
--- a/arch/ia64/include/asm/fpswa.h
+++ b/arch/ia64/include/asm/fpswa.h
@@ -68,6 +68,7 @@ typedef struct {
efi_fpswa_t fpswa;
} fpswa_interface_t;
+#ifdef __KERNEL__
extern fpswa_interface_t *fpswa_interface;
-
+#endif
#endif /* _ASM_IA64_FPSWA_H */
diff --git a/arch/m68k/include/asm/swab.h b/arch/m68k/include/asm/swab.h
index 9e3054e..5b754aa 100644
--- a/arch/m68k/include/asm/swab.h
+++ b/arch/m68k/include/asm/swab.h
@@ -1,7 +1,7 @@
#ifndef _M68K_SWAB_H
#define _M68K_SWAB_H
-#include <asm/types.h>
+#include <linux/types.h>
#include <linux/compiler.h>
#define __SWAB_64_THRU_32__
diff --git a/arch/mips/include/asm/ioctl.h b/arch/mips/include/asm/ioctl.h
index 85067e2..9161634 100644
--- a/arch/mips/include/asm/ioctl.h
+++ b/arch/mips/include/asm/ioctl.h
@@ -60,12 +60,16 @@
((nr) << _IOC_NRSHIFT) | \
((size) << _IOC_SIZESHIFT))
+#ifdef __KERNEL__
/* provoke compile error for invalid uses of size argument */
extern unsigned int __invalid_size_argument_for_IOC;
#define _IOC_TYPECHECK(t) \
((sizeof(t) == sizeof(t[1]) && \
sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
sizeof(t) : __invalid_size_argument_for_IOC)
+#else
+#define _IOC_TYPECHECK(t) (sizeof(t))
+#endif
/* used to create numbers */
#define _IO(type, nr) _IOC(_IOC_NONE, (type), (nr), 0)
diff --git a/arch/mn10300/include/asm/ptrace.h b/arch/mn10300/include/asm/ptrace.h
index 7b06cc6..00aa680 100644
--- a/arch/mn10300/include/asm/ptrace.h
+++ b/arch/mn10300/include/asm/ptrace.h
@@ -77,8 +77,6 @@ struct pt_regs {
};
#endif
-extern struct pt_regs *__frame; /* current frame pointer */
-
/* Arbitrarily choose the same ptrace numbers as used by the Sparc code. */
#define PTRACE_GETREGS 12
#define PTRACE_SETREGS 13
@@ -90,6 +88,8 @@ extern struct pt_regs *__frame; /* current frame pointer */
#if defined(__KERNEL__)
+extern struct pt_regs *__frame; /* current frame pointer */
+
#if !defined(__ASSEMBLY__)
#define user_mode(regs) (((regs)->epsw & EPSW_nSL) == EPSW_nSL)
#define instruction_pointer(regs) ((regs)->pc)
@@ -99,5 +99,4 @@ extern void show_regs(struct pt_regs *);
#define profile_pc(regs) ((regs)->pc)
#endif /* __KERNEL__ */
-
#endif /* _ASM_PTRACE_H */
diff --git a/arch/mn10300/include/asm/setup.h b/arch/mn10300/include/asm/setup.h
index 08356c8..c229d1e 100644
--- a/arch/mn10300/include/asm/setup.h
+++ b/arch/mn10300/include/asm/setup.h
@@ -11,7 +11,8 @@
#ifndef _ASM_SETUP_H
#define _ASM_SETUP_H
+#ifdef __KERNEL__
extern void __init unit_setup(void);
extern void __init unit_init_IRQ(void);
-
+#endif
#endif /* _ASM_SETUP_H */
On Sat, Jun 06, 2009 at 06:24:17PM +0530, Jaswinder Singh Rajput wrote:
> Linus,
>
> Please pull headers_check fixes:
>
> The following changes since commit 064e38aaded5269e573ac1c765284fd65c8ebd13:
> Linus Torvalds (1):
> Merge git://git.kernel.org/.../mason/btrfs-unstable
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/headers-check-2.6.git master
>
> Jaswinder Singh Rajput (6):
> headers_check fix: arm, hwcap.h
So inspite of NAKing this patch you send it anyway.
Linus, please don't pull this.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Sat, 2009-06-06 at 14:02 +0100, Russell King wrote:
> On Sat, Jun 06, 2009 at 06:24:17PM +0530, Jaswinder Singh Rajput wrote:
> > Linus,
> >
> > Please pull headers_check fixes:
> >
> > The following changes since commit 064e38aaded5269e573ac1c765284fd65c8ebd13:
> > Linus Torvalds (1):
> > Merge git://git.kernel.org/.../mason/btrfs-unstable
> >
> > are available in the git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/headers-check-2.6.git master
> >
> > Jaswinder Singh Rajput (6):
> > headers_check fix: arm, hwcap.h
>
> So inspite of NAKing this patch you send it anyway.
>
Earlier it was already like this, because of Catalin Marinas
fault (commit f884b1cf578e0) and your ignorance you converted :
#ifdef __KERNEL__
#ifndef __ASSEMBLY__
to :
#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
I am just trying to rectify your mistake that __KERNEL__ should not be
used with other defines to easily export headers to userspace. Whole
kernel header files are following this protocol. Sam, Arnd and me are
trying to convince you. Why you need an exception ?
Thanks,
--
JSR
On Sat, Jun 06, 2009 at 07:04:37PM +0530, Jaswinder Singh Rajput wrote:
> On Sat, 2009-06-06 at 14:02 +0100, Russell King wrote:
> > On Sat, Jun 06, 2009 at 06:24:17PM +0530, Jaswinder Singh Rajput wrote:
> > > Linus,
> > >
> > > Please pull headers_check fixes:
> > >
> > > The following changes since commit 064e38aaded5269e573ac1c765284fd65c8ebd13:
> > > Linus Torvalds (1):
> > > Merge git://git.kernel.org/.../mason/btrfs-unstable
> > >
> > > are available in the git repository at:
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/headers-check-2.6.git master
> > >
> > > Jaswinder Singh Rajput (6):
> > > headers_check fix: arm, hwcap.h
> >
> > So inspite of NAKing this patch you send it anyway.
> >
>
> Earlier it was already like this, because of Catalin Marinas
> fault (commit f884b1cf578e0) and your ignorance you converted :
>
> #ifdef __KERNEL__
> #ifndef __ASSEMBLY__
>
> to :
>
> #if defined(__KERNEL__) && !defined(__ASSEMBLY__)
>
> I am just trying to rectify your mistake that __KERNEL__ should not be
> used with other defines to easily export headers to userspace. Whole
> kernel header files are following this protocol. Sam, Arnd and me are
> trying to convince you. Why you need an exception ?
You are not listening.
I believe that there is a much better way to fix this, and that is to
fix the tools. Having looked at unifdef, I believe that the fix is
almost trivial in nature.
With fixed tools, we don't have to fuck around writing a special dialect
of C to work around their short comings - a dialect which isn't obvious.
What we actually have here is a small number of people who've suddenly
decided that a perfectly good bit of C code is no longer good enough for
their purposes, and are now requiring things to be written to match their
own rules.
Let's fix the tools and then we don't have to ever worry about this
problem again, or require people to write their C code in special ways.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Sat, 2009-06-06 at 14:02 +0100, Russell King wrote:
> On Sat, Jun 06, 2009 at 06:24:17PM +0530, Jaswinder Singh Rajput wrote:
> > Linus,
> >
> > Please pull headers_check fixes:
> >
> > The following changes since commit 064e38aaded5269e573ac1c765284fd65c8ebd13:
> > Linus Torvalds (1):
> > Merge git://git.kernel.org/.../mason/btrfs-unstable
> >
> > are available in the git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/headers-check-2.6.git master
> >
> > Jaswinder Singh Rajput (6):
> > headers_check fix: arm, hwcap.h
>
> So inspite of NAKing this patch you send it anyway.
>
Ok dropping this patch as this issue is still open.
Here is new pull request:
The following changes since commit 064e38aaded5269e573ac1c765284fd65c8ebd13:
Linus Torvalds (1):
Merge git://git.kernel.org/.../mason/btrfs-unstable
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/headers-check-2.6.git master
Jaswinder Singh Rajput (5):
headers_check fix: ia64, fpswa.h
headers_check fix: m68k, swab.h
headers_check fix: mips, ioctl.h
headers_check fix: mn10300, ptrace.h
headers_check fix: mn10300, setup.h
arch/ia64/include/asm/fpswa.h | 3 ++-
arch/m68k/include/asm/swab.h | 2 +-
arch/mips/include/asm/ioctl.h | 4 ++++
arch/mn10300/include/asm/ptrace.h | 5 ++---
arch/mn10300/include/asm/setup.h | 3 ++-
5 files changed, 11 insertions(+), 6 deletions(-)
Complete diff :
diff --git a/arch/ia64/include/asm/fpswa.h b/arch/ia64/include/asm/fpswa.h
index 62edfce..797b064 100644
--- a/arch/ia64/include/asm/fpswa.h
+++ b/arch/ia64/include/asm/fpswa.h
@@ -68,6 +68,7 @@ typedef struct {
efi_fpswa_t fpswa;
} fpswa_interface_t;
+#ifdef __KERNEL__
extern fpswa_interface_t *fpswa_interface;
-
+#endif
#endif /* _ASM_IA64_FPSWA_H */
diff --git a/arch/m68k/include/asm/swab.h b/arch/m68k/include/asm/swab.h
index 9e3054e..5b754aa 100644
--- a/arch/m68k/include/asm/swab.h
+++ b/arch/m68k/include/asm/swab.h
@@ -1,7 +1,7 @@
#ifndef _M68K_SWAB_H
#define _M68K_SWAB_H
-#include <asm/types.h>
+#include <linux/types.h>
#include <linux/compiler.h>
#define __SWAB_64_THRU_32__
diff --git a/arch/mips/include/asm/ioctl.h b/arch/mips/include/asm/ioctl.h
index 85067e2..9161634 100644
--- a/arch/mips/include/asm/ioctl.h
+++ b/arch/mips/include/asm/ioctl.h
@@ -60,12 +60,16 @@
((nr) << _IOC_NRSHIFT) | \
((size) << _IOC_SIZESHIFT))
+#ifdef __KERNEL__
/* provoke compile error for invalid uses of size argument */
extern unsigned int __invalid_size_argument_for_IOC;
#define _IOC_TYPECHECK(t) \
((sizeof(t) == sizeof(t[1]) && \
sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
sizeof(t) : __invalid_size_argument_for_IOC)
+#else
+#define _IOC_TYPECHECK(t) (sizeof(t))
+#endif
/* used to create numbers */
#define _IO(type, nr) _IOC(_IOC_NONE, (type), (nr), 0)
diff --git a/arch/mn10300/include/asm/ptrace.h b/arch/mn10300/include/asm/ptrace.h
index 7b06cc6..00aa680 100644
--- a/arch/mn10300/include/asm/ptrace.h
+++ b/arch/mn10300/include/asm/ptrace.h
@@ -77,8 +77,6 @@ struct pt_regs {
};
#endif
-extern struct pt_regs *__frame; /* current frame pointer */
-
/* Arbitrarily choose the same ptrace numbers as used by the Sparc code. */
#define PTRACE_GETREGS 12
#define PTRACE_SETREGS 13
@@ -90,6 +88,8 @@ extern struct pt_regs *__frame; /* current frame pointer */
#if defined(__KERNEL__)
+extern struct pt_regs *__frame; /* current frame pointer */
+
#if !defined(__ASSEMBLY__)
#define user_mode(regs) (((regs)->epsw & EPSW_nSL) == EPSW_nSL)
#define instruction_pointer(regs) ((regs)->pc)
@@ -99,5 +99,4 @@ extern void show_regs(struct pt_regs *);
#define profile_pc(regs) ((regs)->pc)
#endif /* __KERNEL__ */
-
#endif /* _ASM_PTRACE_H */
diff --git a/arch/mn10300/include/asm/setup.h b/arch/mn10300/include/asm/setup.h
index 08356c8..c229d1e 100644
--- a/arch/mn10300/include/asm/setup.h
+++ b/arch/mn10300/include/asm/setup.h
@@ -11,7 +11,8 @@
#ifndef _ASM_SETUP_H
#define _ASM_SETUP_H
+#ifdef __KERNEL__
extern void __init unit_setup(void);
extern void __init unit_init_IRQ(void);
-
+#endif
#endif /* _ASM_SETUP_H */
Right, below is a patch to unifdef.c which allows it to work out if
an #if expression always evaluates true or false for symbols which
are being undefined/always defined.
The patch is slightly more complicated than I'd hoped because unifdef
needs to see lines fully evaluated - doing otherwise causes it to
mark the line as "dirty" and copy it over no matter what.
What follows this email is the diff of what effect it has on the
headers copied over - as can be seen, all resulting changes are of
net benefit.
Signed-off-by: Russell King <[email protected]>
diff --git a/scripts/unifdef.c b/scripts/unifdef.c
index 05a31a6..30d459f 100644
--- a/scripts/unifdef.c
+++ b/scripts/unifdef.c
@@ -678,8 +678,10 @@ eval_unary(const struct ops *ops, int *valp, const char **cpp)
if (*cp == '!') {
debug("eval%d !", ops - eval_ops);
cp++;
- if (eval_unary(ops, valp, &cp) == LT_IF)
+ if (eval_unary(ops, valp, &cp) == LT_IF) {
+ *cpp = cp;
return (LT_IF);
+ }
*valp = !*valp;
} else if (*cp == '(') {
cp++;
@@ -700,13 +702,16 @@ eval_unary(const struct ops *ops, int *valp, const char **cpp)
return (LT_IF);
cp = skipcomment(cp);
sym = findsym(cp);
- if (sym < 0)
- return (LT_IF);
- *valp = (value[sym] != NULL);
cp = skipsym(cp);
cp = skipcomment(cp);
if (*cp++ != ')')
return (LT_IF);
+ if (sym >= 0)
+ *valp = (value[sym] != NULL);
+ else {
+ *cpp = cp;
+ return (LT_IF);
+ }
keepthis = false;
} else if (!endsym(*cp)) {
debug("eval%d symbol", ops - eval_ops);
@@ -741,11 +746,11 @@ eval_table(const struct ops *ops, int *valp, const char **cpp)
const struct op *op;
const char *cp;
int val;
+ Linetype lhs, rhs;
debug("eval%d", ops - eval_ops);
cp = *cpp;
- if (ops->inner(ops+1, valp, &cp) == LT_IF)
- return (LT_IF);
+ lhs = ops->inner(ops+1, valp, &cp);
for (;;) {
cp = skipcomment(cp);
for (op = ops->op; op->str != NULL; op++)
@@ -755,14 +760,32 @@ eval_table(const struct ops *ops, int *valp, const char **cpp)
break;
cp += strlen(op->str);
debug("eval%d %s", ops - eval_ops, op->str);
- if (ops->inner(ops+1, &val, &cp) == LT_IF)
- return (LT_IF);
- *valp = op->fn(*valp, val);
+ rhs = ops->inner(ops+1, &val, &cp);
+ if (op->fn == op_and && (lhs == LT_FALSE || rhs == LT_FALSE)) {
+ debug("eval%d: and always false", ops - eval_ops);
+ if (lhs == LT_IF)
+ *valp = val;
+ lhs = LT_FALSE;
+ continue;
+ }
+ if (op->fn == op_or && (lhs == LT_TRUE || rhs == LT_TRUE)) {
+ debug("eval%d: or always true", ops - eval_ops);
+ if (lhs == LT_IF)
+ *valp = val;
+ lhs = LT_TRUE;
+ continue;
+ }
+ if (rhs == LT_IF)
+ lhs = LT_IF;
+ if (lhs != LT_IF)
+ *valp = op->fn(*valp, val);
}
*cpp = cp;
debug("eval%d = %d", ops - eval_ops, *valp);
- return (*valp ? LT_TRUE : LT_FALSE);
+ if (lhs != LT_IF)
+ lhs = (*valp ? LT_TRUE : LT_FALSE);
+ return lhs;
}
/*
@@ -773,12 +796,15 @@ eval_table(const struct ops *ops, int *valp, const char **cpp)
static Linetype
ifeval(const char **cpp)
{
+ const char *cp = *cpp;
int ret;
int val;
debug("eval %s", *cpp);
keepthis = killconsts ? false : true;
- ret = eval_table(eval_ops, &val, cpp);
+ ret = eval_table(eval_ops, &val, &cp);
+ if (ret != LT_IF)
+ *cpp = cp;
debug("eval = %d", val);
return (keepthis ? LT_IF : ret);
}
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Sat, Jun 06, 2009 at 10:47:11PM +0100, Russell King wrote:
> What follows this email is the diff of what effect it has on the
> headers copied over - as can be seen, all resulting changes are of
> net benefit.
And here is the patch illustrating the overall change on the installed
header files. As can be seen, all instances of:
#if defined(__KERNEL__) && ...
...
#endif
are removed, and:
#if !defined(__KERNEL__) || ...
...
#endif
have the conditionals also correctly removed, leaving the code between
properly exposed.
Clearly from the number of differences below, I'm not the only one who
is "ignorant" of this magic new rule about __KERNEL__, and I suggest
that no such rule actually ever existed.
diff -ur include.old/asm/hwcap.h include/asm/hwcap.h
--- include.old/asm/hwcap.h 2009-06-06 19:29:31.000000000 +0100
+++ include/asm/hwcap.h 2009-06-06 22:40:58.000000000 +0100
@@ -20,13 +20,5 @@
#define HWCAP_VFPv3 8192
#define HWCAP_VFPv3D16 16384
-#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
-/*
- * This yields a mask that user programs can use to figure out what
- * instruction set this cpu supports.
- */
-#define ELF_HWCAP (elf_hwcap)
-extern unsigned int elf_hwcap;
-#endif
#endif
diff -ur include.old/linux/acct.h include/linux/acct.h
--- include.old/linux/acct.h 2009-06-06 19:29:27.000000000 +0100
+++ include/linux/acct.h 2009-06-06 22:40:55.000000000 +0100
@@ -59,9 +59,7 @@
comp_t ac_majflt; /* Major Pagefaults */
comp_t ac_swaps; /* Number of Swaps */
/* m68k had no padding here. */
-#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
__u16 ac_ahz; /* AHZ */
-#endif
__u32 ac_exitcode; /* Exitcode */
char ac_comm[ACCT_COMM + 1]; /* Command Name */
__u8 ac_etime_hi; /* Elapsed Time MSB */
diff -ur include.old/linux/soundcard.h include/linux/soundcard.h
--- include.old/linux/soundcard.h 2009-06-06 19:29:30.000000000 +0100
+++ include/linux/soundcard.h 2009-06-06 22:40:57.000000000 +0100
@@ -1033,7 +1033,6 @@
*/
#define LOCL_STARTAUDIO 1
-#if !defined(__KERNEL__) || defined(USE_SEQ_MACROS)
/*
* Some convenience macros to simplify programming of the
* /dev/sequencer interface
@@ -1275,4 +1274,3 @@
(SEQ_DUMPBUF(), write(seqfd, (char*)(patchx), len))
#endif
-#endif
diff -ur include.old/linux/videodev.h include/linux/videodev.h
--- include.old/linux/videodev.h 2009-06-06 19:29:30.000000000 +0100
+++ include/linux/videodev.h 2009-06-06 22:40:58.000000000 +0100
@@ -16,24 +16,6 @@
#include <linux/ioctl.h>
#include <linux/videodev2.h>
-#if defined(__MIN_V4L1) && defined (__KERNEL__)
-
-/*
- * Used by those V4L2 core functions that need a minimum V4L1 support,
- * in order to allow V4L1 Compatibilty code compilation.
- */
-
-struct video_mbuf
-{
- int size; /* Total memory to map */
- int frames; /* Frames */
- int offsets[VIDEO_MAX_FRAME];
-};
-
-#define VIDIOCGMBUF _IOR('v',20, struct video_mbuf) /* Memory map buffer info */
-
-#else
-#if defined(CONFIG_VIDEO_V4L1_COMPAT) || !defined (__KERNEL__)
#define VID_TYPE_CAPTURE 1 /* Can capture */
#define VID_TYPE_TUNER 2 /* Can tune */
@@ -328,8 +310,6 @@
#define VID_PLAY_RESET 13
#define VID_PLAY_END_MARK 14
-#endif /* CONFIG_VIDEO_V4L1_COMPAT */
-#endif /* __MIN_V4L1 */
#endif /* __LINUX_VIDEODEV_H */
diff -ur include.old/video/edid.h include/video/edid.h
--- include.old/video/edid.h 2009-06-06 19:29:31.000000000 +0100
+++ include/video/edid.h 2009-06-06 22:40:58.000000000 +0100
@@ -1,13 +1,11 @@
#ifndef __linux_video_edid_h__
#define __linux_video_edid_h__
-#if !defined(__KERNEL__) || defined(CONFIG_X86)
struct edid_info {
unsigned char dummy[128];
};
-#endif
#endif /* __linux_video_edid_h__ */
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
Hi Russell.
On Sat, Jun 06, 2009 at 10:47:11PM +0100, Russell King wrote:
> Right, below is a patch to unifdef.c which allows it to work out if
> an #if expression always evaluates true or false for symbols which
> are being undefined/always defined.
>
> The patch is slightly more complicated than I'd hoped because unifdef
> needs to see lines fully evaluated - doing otherwise causes it to
> mark the line as "dirty" and copy it over no matter what.
>
> What follows this email is the diff of what effect it has on the
> headers copied over - as can be seen, all resulting changes are of
> net benefit.
You beat me on this one.
I was looking into supportting short_circuit_and/short_circuit_or too.
Your patch looks better - thanks!
>From my very limited testing this patch suffers from a problem I faced too.
When you run it on for example hwcap.h or my sample file:
#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
int a;
#endif
then the return code of the program is 1 so we do not copy the file.
I'm heading for bed - will take a closer look tomorrow.
Sam
On Sun, Jun 07, 2009 at 12:12:23AM +0200, Sam Ravnborg wrote:
> Hi Russell.
>
> On Sat, Jun 06, 2009 at 10:47:11PM +0100, Russell King wrote:
> > Right, below is a patch to unifdef.c which allows it to work out if
> > an #if expression always evaluates true or false for symbols which
> > are being undefined/always defined.
> >
> > The patch is slightly more complicated than I'd hoped because unifdef
> > needs to see lines fully evaluated - doing otherwise causes it to
> > mark the line as "dirty" and copy it over no matter what.
> >
> > What follows this email is the diff of what effect it has on the
> > headers copied over - as can be seen, all resulting changes are of
> > net benefit.
>
> You beat me on this one.
> I was looking into supportting short_circuit_and/short_circuit_or too.
> Your patch looks better - thanks!
>
> >From my very limited testing this patch suffers from a problem I faced too.
> When you run it on for example hwcap.h or my sample file:
> #if defined(__KERNEL__) && !defined(__ASSEMBLY__)
> int a;
> #endif
>
> then the return code of the program is 1 so we do not copy the file.
That's correct. If unifdef removes any lines, the exit code is 1, not
zero. This happens with the unmodified unifdef as well, so I'm not
changing that behaviour.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Sun, Jun 07, 2009 at 12:24:15AM +0100, Russell King wrote:
> On Sun, Jun 07, 2009 at 12:12:23AM +0200, Sam Ravnborg wrote:
> > Hi Russell.
> >
> > On Sat, Jun 06, 2009 at 10:47:11PM +0100, Russell King wrote:
> > > Right, below is a patch to unifdef.c which allows it to work out if
> > > an #if expression always evaluates true or false for symbols which
> > > are being undefined/always defined.
> > >
> > > The patch is slightly more complicated than I'd hoped because unifdef
> > > needs to see lines fully evaluated - doing otherwise causes it to
> > > mark the line as "dirty" and copy it over no matter what.
> > >
> > > What follows this email is the diff of what effect it has on the
> > > headers copied over - as can be seen, all resulting changes are of
> > > net benefit.
> >
> > You beat me on this one.
> > I was looking into supportting short_circuit_and/short_circuit_or too.
> > Your patch looks better - thanks!
> >
> > >From my very limited testing this patch suffers from a problem I faced too.
> > When you run it on for example hwcap.h or my sample file:
> > #if defined(__KERNEL__) && !defined(__ASSEMBLY__)
> > int a;
> > #endif
> >
> > then the return code of the program is 1 so we do not copy the file.
>
> That's correct. If unifdef removes any lines, the exit code is 1, not
> zero. This happens with the unmodified unifdef as well, so I'm not
> changing that behaviour.
Thanks.
I was just fooled in my testing.
With the same file I saw two different return codes with and
without the patch. But this is because you fixed unifdef.
Sam
On Sat, Jun 06, 2009 at 10:47:11PM +0100, Russell King wrote:
> Right, below is a patch to unifdef.c which allows it to work out if
> an #if expression always evaluates true or false for symbols which
> are being undefined/always defined.
>
> The patch is slightly more complicated than I'd hoped because unifdef
> needs to see lines fully evaluated - doing otherwise causes it to
> mark the line as "dirty" and copy it over no matter what.
>
> What follows this email is the diff of what effect it has on the
> headers copied over - as can be seen, all resulting changes are of
> net benefit.
Result is great - thanks for implmenting this!
I have applied the following (no code changes, only a few more
details in the changelog).
Sam
commit 3a8a10760f9626856848ae815e9436a2e04a4082
Author: Russell King <[email protected]>
Date: Sat Jun 6 22:47:11 2009 +0100
kbuild: fix headers_exports with boolean expression
When we had code like this in a header unifdef failed to
deduct that the expression was always false - and we had code exported
that was not intended for userspace.
#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
int a;
#endif
This commit implment support in unidef which allows it to work out if
an #if expression always evaluates true or false for symbols which
are being undefined/always defined.
The patch is slightly more complicated than I'd hoped because unifdef
needs to see lines fully evaluated - doing otherwise causes it to
mark the line as "dirty" and copy it over no matter what.
Signed-off-by: Russell King <[email protected]>
Signed-off-by: Sam Ravnborg <[email protected]>
diff --git a/scripts/unifdef.c b/scripts/unifdef.c
index 05a31a6..30d459f 100644
--- a/scripts/unifdef.c
+++ b/scripts/unifdef.c
@@ -678,8 +678,10 @@ eval_unary(const struct ops *ops, int *valp, const char **cpp)
if (*cp == '!') {
debug("eval%d !", ops - eval_ops);
cp++;
- if (eval_unary(ops, valp, &cp) == LT_IF)
+ if (eval_unary(ops, valp, &cp) == LT_IF) {
+ *cpp = cp;
return (LT_IF);
+ }
*valp = !*valp;
} else if (*cp == '(') {
cp++;
@@ -700,13 +702,16 @@ eval_unary(const struct ops *ops, int *valp, const char **cpp)
return (LT_IF);
cp = skipcomment(cp);
sym = findsym(cp);
- if (sym < 0)
- return (LT_IF);
- *valp = (value[sym] != NULL);
cp = skipsym(cp);
cp = skipcomment(cp);
if (*cp++ != ')')
return (LT_IF);
+ if (sym >= 0)
+ *valp = (value[sym] != NULL);
+ else {
+ *cpp = cp;
+ return (LT_IF);
+ }
keepthis = false;
} else if (!endsym(*cp)) {
debug("eval%d symbol", ops - eval_ops);
@@ -741,11 +746,11 @@ eval_table(const struct ops *ops, int *valp, const char **cpp)
const struct op *op;
const char *cp;
int val;
+ Linetype lhs, rhs;
debug("eval%d", ops - eval_ops);
cp = *cpp;
- if (ops->inner(ops+1, valp, &cp) == LT_IF)
- return (LT_IF);
+ lhs = ops->inner(ops+1, valp, &cp);
for (;;) {
cp = skipcomment(cp);
for (op = ops->op; op->str != NULL; op++)
@@ -755,14 +760,32 @@ eval_table(const struct ops *ops, int *valp, const char **cpp)
break;
cp += strlen(op->str);
debug("eval%d %s", ops - eval_ops, op->str);
- if (ops->inner(ops+1, &val, &cp) == LT_IF)
- return (LT_IF);
- *valp = op->fn(*valp, val);
+ rhs = ops->inner(ops+1, &val, &cp);
+ if (op->fn == op_and && (lhs == LT_FALSE || rhs == LT_FALSE)) {
+ debug("eval%d: and always false", ops - eval_ops);
+ if (lhs == LT_IF)
+ *valp = val;
+ lhs = LT_FALSE;
+ continue;
+ }
+ if (op->fn == op_or && (lhs == LT_TRUE || rhs == LT_TRUE)) {
+ debug("eval%d: or always true", ops - eval_ops);
+ if (lhs == LT_IF)
+ *valp = val;
+ lhs = LT_TRUE;
+ continue;
+ }
+ if (rhs == LT_IF)
+ lhs = LT_IF;
+ if (lhs != LT_IF)
+ *valp = op->fn(*valp, val);
}
*cpp = cp;
debug("eval%d = %d", ops - eval_ops, *valp);
- return (*valp ? LT_TRUE : LT_FALSE);
+ if (lhs != LT_IF)
+ lhs = (*valp ? LT_TRUE : LT_FALSE);
+ return lhs;
}
/*
@@ -773,12 +796,15 @@ eval_table(const struct ops *ops, int *valp, const char **cpp)
static Linetype
ifeval(const char **cpp)
{
+ const char *cp = *cpp;
int ret;
int val;
debug("eval %s", *cpp);
keepthis = killconsts ? false : true;
- ret = eval_table(eval_ops, &val, cpp);
+ ret = eval_table(eval_ops, &val, &cp);
+ if (ret != LT_IF)
+ *cpp = cp;
debug("eval = %d", val);
return (keepthis ? LT_IF : ret);
}
Hello Russell,
On Sat, 2009-06-06 at 22:47 +0100, Russell King wrote:
> Right, below is a patch to unifdef.c which allows it to work out if
> an #if expression always evaluates true or false for symbols which
> are being undefined/always defined.
>
> The patch is slightly more complicated than I'd hoped because unifdef
> needs to see lines fully evaluated - doing otherwise causes it to
> mark the line as "dirty" and copy it over no matter what.
>
> What follows this email is the diff of what effect it has on the
> headers copied over - as can be seen, all resulting changes are of
> net benefit.
>
> Signed-off-by: Russell King <[email protected]>
>
This patch not only solve ARM's hwcap.h but also fixes some other arch
header files.
Great work, thanks.
--
JSR
> Tony - is this file really used from userspace.
> Looking at the definitions I think not.
>
> I not then the real fix is to unexport it.
Sam,
FPSWA should be completely invisible to user space. You are correct that the
right fix is to unexport it.
-Tony
fpswa.h is not relevant for userspace,
so do not export it.
Signed-off-by: Sam Ravnborg <[email protected]>
Cc: Jaswinder Singh Rajput <[email protected]>
---
diff --git a/arch/ia64/include/asm/Kbuild b/arch/ia64/include/asm/Kbuild
index ccbe8ae..c7d0a71 100644
--- a/arch/ia64/include/asm/Kbuild
+++ b/arch/ia64/include/asm/Kbuild
@@ -2,7 +2,6 @@ include include/asm-generic/Kbuild.asm
header-y += break.h
header-y += fpu.h
-header-y += fpswa.h
header-y += ia64regs.h
header-y += intel_intrin.h
header-y += perfmon_default_smpl.h