Re: [PATCH] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h

Re: [PATCH] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h

[Palmer Dabbelt]

I cut the RISC-V stuff, but I intend to reply to it later.  As you
said, it's just a different topic.

>>> However, I did see a lot of similar bugs now that you point me to it:
>>>
>>> $  grep -r \\\<CONFIG obj-tmp/usr/include/
>>> obj-tmp/usr/include/asm-generic/fcntl.h:#ifndef CONFIG_64BIT
>>> obj-tmp/usr/include/asm-generic/mman-common.h:#ifdef CONFIG_MMAP_ALLOW_UNINITIALIZED
>>> obj-tmp/usr/include/asm-generic/unistd.h:#ifdef CONFIG_MMU
>>> obj-tmp/usr/include/asm-generic/unistd.h:#endif /* CONFIG_MMU */
>>> obj-tmp/usr/include/linux/atmdev.h:#ifdef CONFIG_COMPAT
>>> obj-tmp/usr/include/linux/elfcore.h:#ifdef CONFIG_BINFMT_ELF_FDPIC
>>> obj-tmp/usr/include/linux/eventpoll.h:#ifdef CONFIG_PM_SLEEP
>>> obj-tmp/usr/include/linux/fb.h:#ifdef CONFIG_FB_BACKLIGHT
>>> obj-tmp/usr/include/linux/flat.h:#ifdef CONFIG_BINFMT_SHARED_FLAT
>>> obj-tmp/usr/include/linux/hw_breakpoint.h:#ifdef CONFIG_HAVE_MIXED_BREAKPOINTS_REGS
>>> obj-tmp/usr/include/linux/pktcdvd.h:#if defined(CONFIG_CDROM_PKTCDVD_WCACHE)
>>> obj-tmp/usr/include/linux/raw.h:#define MAX_RAW_MINORS CONFIG_MAX_RAW_DEVS
>>> obj-tmp/usr/include/asm/ptrace.h:#ifdef CONFIG_CPU_ENDIAN_BE8
>>>
>>> These all have the same problem, and we should fix them, as well as
>>> (probably) adding an automated check to scripts/headers_install.sh.
>>
>> Well, I was going to go fix them all and ran a very similar grep, but
>> I think I got a lot of false-positives.  If I understand correctly,
>> it's allowed to have CONFIG_* when guarded by __KERNEL__ in
>> user-visible headers?
>
> That is right.

It turns out there was actually a header checking script
(scripts/headers_check.pl), and it already had a check for this.  The
check was just disabled because there was "too much noise".  Rather
than putting it in headers_install I've just fixed that script.  I'm
definately lacking in perl powers, so I have no idea if what I've done
is sane.  Specifically: there's a global variable and a line over 80
characters, but since there's a bunch of other violations I figure
it's fine.

>> Now that I've written that, I realize it'd be pretty easy to just use
>> cpp to drop everything inside __KERNEL__ and then look for CONFIG_*.
>
> The lines quoted above are from the output of 'make headers_install',
> which already drops everything inside of __KERNEL__. A lot of them
> probably just need to add that #ifdef, or move the portion of the
> header file to the normal (non-uabi) file.
>
>> If you want, I can try to do that, fix what triggers the check, and
>> re-submit everything together?
>
> That would be great, yes.

OK.  I think this has turned into more of a RFC than a PATCH,
though...  I've just #ifdef'd things for now to reduce the diff size,
though I think it might be cleaner to move some of them to the
non-user headers (ep_take_care_of_epollwakeup(), USE_WCACHING,
MAX_RAW_MINORS).

I'm pretty far out of my depth here, so these should all be carefully
considered, but there's a few that scare me more ("struct
elf_prstatus", "enum by_type_idx", AT_VECTOR_SIZE_ARCH).  I think
there's only one actual bug here (MAP_UNINITIALIZED), the rest just
quiet the checking script.  Each patch has my rationale for what I
did.

Since this touches a whole lot of stuff, I've added a whole bunch of
CCs.

[PATCH 01/13] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h

[Palmer Dabbelt]

When working on the RISC-V port I noticed that F_SETLK64 was being
defined on our 64-bit platform, despite our port being so new that
we've only ever had the 64-bit file ops.  Since there's not compat
layer for these, this causes fcntl to bail out.

It turns out that one of the ways in with F_SETLK64 was being defined
(there's some more in glibc, but that's a whole different story... :))
is the result of CONFIG_64BIT showing up in this user-visible header.
<asm-generic/bitsperlong.h> confirms this isn't sane, so I replaced it
with a __BITS_PER_LONG check.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/asm-generic/fcntl.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index e063effe0cc1..14a5c8237d84 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -1,6 +1,7 @@
 #ifndef _ASM_GENERIC_FCNTL_H
 #define _ASM_GENERIC_FCNTL_H
 
+#include <asm/bitsperlong.h>
 #include <linux/types.h>
 
 /*
@@ -115,7 +116,7 @@
 #define F_GETSIG	11	/* for sockets. */
 #endif
 
-#ifndef CONFIG_64BIT
+#if (__BITS_PER_LONG = 32)
 #ifndef F_GETLK64
 #define F_GETLK64	12	/*  using 'struct flock64' */
 #define F_SETLK64	13
-- 
2.4.6

[PATCH 02/13] Always expose __SYSCALL(... fork ...)

[Palmer Dabbelt]

I think this change actually doesn't do anything: __NR_fork was still
being defined either way, and on my machine fork() in <unistd.h> comes
from libc.

That said, I don't think there's any way to determine this
automatically, so this at least quiets the checker.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/asm-generic/unistd.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index e016bd9b1a04..e027ef7aa01f 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -865,11 +865,11 @@ __SYSCALL(__NR_uselib, sys_uselib)
 __SYSCALL(__NR__sysctl, sys_sysctl)
 
 #define __NR_fork 1079
-#ifdef CONFIG_MMU
+#if !defined(__KERNEL__) || defined(CONFIG_MMU)
 __SYSCALL(__NR_fork, sys_fork)
 #else
 __SYSCALL(__NR_fork, sys_ni_syscall)
-#endif /* CONFIG_MMU */
+#endif /* !__KERNEL__ || CONFIG_MMU */
 
 #undef __NR_syscalls
 #define __NR_syscalls (__NR_fork+1)
-- 
2.4.6

[PATCH 03/13] Hide COMPAT_ATM_ADDPARTY behind #ifdef __KERNEL__

[Palmer Dabbelt]

This used to just be behind an #ifdef COMPAT_COMPAT, so most of
userspace wouldn't have seen the definition before.  This change just
makes the __KERNEL__ part explicit to quiet the header checker.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/atmdev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/atmdev.h b/include/uapi/linux/atmdev.h
index 93e0ec008ca8..f8b6223165da 100644
--- a/include/uapi/linux/atmdev.h
+++ b/include/uapi/linux/atmdev.h
@@ -100,7 +100,7 @@ struct atm_dev_stats {
 					/* use backend to make new if */
 #define ATM_ADDPARTY  	_IOW('a', ATMIOC_SPECIAL+4,struct atm_iobuf)
  					/* add party to p2mp call */
-#ifdef CONFIG_COMPAT
+#if defined(__KERNEL__) && defined(CONFIG_COMPAT)
 /* It actually takes struct sockaddr_atmsvc, not struct atm_iobuf */
 #define COMPAT_ATM_ADDPARTY  	_IOW('a', ATMIOC_SPECIAL+4,struct compat_atm_iobuf)
 #endif
-- 
2.4.6

[PATCH 04/13] Always expose MAP_UNINITIALIZED to userspace

[Palmer Dabbelt]

This used to be hidden behind CONFIG_MMAP_ALLOW_UNINITIALIZED, so
userspace wouldn't actually ever see it.  While I could have kept
hiding it, the man pages seem to indicate that MAP_UNINITIALIZED
should be visible:

  mmap(2)
  MAP_UNINITIALIZED (since Linux 2.6.33)
    Don't clear anonymous pages.  This flag is intended to improve
    performance on embedded devices.  This flag is honored only if the
    kernel was configured with the CONFIG_MMAP_ALLOW_UNINITIALIZED
    option.  Because of the security implications, that option is
    normally enabled only on embedded devices (i.e., devices where one
    has complete control of the contents of user memory).

and since the only time it shows up in my /usr/include is in this
header I believe this should have been visible to userspace (as
non-zero, which wouldn't do anything when or'd into the flags) all
along.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/asm-generic/mman-common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index ddc3b36f1046..e58d1911ecc6 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -19,7 +19,7 @@
 #define MAP_TYPE	0x0f		/* Mask for type of mapping */
 #define MAP_FIXED	0x10		/* Interpret addr exactly */
 #define MAP_ANONYMOUS	0x20		/* don't use a file */
-#ifdef CONFIG_MMAP_ALLOW_UNINITIALIZED
+#if !defined(__KERNEL__) || defined(CONFIG_MMAP_ALLOW_UNINITIALIZED)
 # define MAP_UNINITIALIZED 0x4000000	/* For anonymous mmap, memory could be uninitialized */
 #else
 # define MAP_UNINITIALIZED 0x0		/* Don't support this flag */
-- 
2.4.6

[PATCH 05/13] Hide some of "struct elf_prstatus" behind #ifdef __KERNEL__

[Palmer Dabbelt]

This one scares me: while I can't find any system calls that directly
take this as an argument, a comment in <linux/ptrace.h>

  "
   Generic ptrace interface that exports the architecture specific
   regsets using the corresponding NT_* types (which are also used in
   the core dump).  Please note that the NT_PRSTATUS note type in a
   core dump contains a full 'struct elf_prstatus'. But the
   user_regset for NT_PRSTATUS contains just the elf_gregset_t that is
   the pr_reg field of 'struct elf_prstatus'. For all the other
   user_regset flavors, the user_regset layout and the ELF core dump
   note payload are exactly the same layout.
  "

seems to indicate that it's possible to see this sometimes.  Since
this would only be visible to userspace in a somewhat convoluted
manner, I'm going to try and keep it as it was.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/elfcore.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/elfcore.h b/include/uapi/linux/elfcore.h
index 569737cfb557..f9320b588937 100644
--- a/include/uapi/linux/elfcore.h
+++ b/include/uapi/linux/elfcore.h
@@ -60,7 +60,7 @@ struct elf_prstatus
 	long	pr_instr;		/* Current instruction */
 #endif
 	elf_gregset_t pr_reg;	/* GP registers */
-#ifdef CONFIG_BINFMT_ELF_FDPIC
+#if defined(__KERNEL__) && defined(CONFIG_BINFMT_ELF_FDPIC)
 	/* When using FDPIC, the loadmap addresses need to be communicated
 	 * to GDB in order for GDB to do the necessary relocations.  The
 	 * fields (below) used to communicate this information are placed
-- 
2.4.6

[PATCH 06/13] Hide ep_take_care_of_epollwakeup() behind #ifdef __KERNEL__

[Palmer Dabbelt]

This doesn't make any sense to expose to userspace.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/eventpoll.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/eventpoll.h b/include/uapi/linux/eventpoll.h
index bc81fb2e1f0e..290426bfb0aa 100644
--- a/include/uapi/linux/eventpoll.h
+++ b/include/uapi/linux/eventpoll.h
@@ -61,6 +61,7 @@ struct epoll_event {
 	__u64 data;
 } EPOLL_PACKED;
 
+#ifdef __KERNEL__
 #ifdef CONFIG_PM_SLEEP
 static inline void ep_take_care_of_epollwakeup(struct epoll_event *epev)
 {
@@ -73,4 +74,6 @@ static inline void ep_take_care_of_epollwakeup(struct epoll_event *epev)
 	epev->events &= ~EPOLLWAKEUP;
 }
 #endif
+#endif /*__KERNEL__*/
+
 #endif /* _UAPI_LINUX_EVENTPOLL_H */
-- 
2.4.6

[PATCH 07/13] Make FB_BACKLIGHT_{LEVELS,MAX} always visible

[Palmer Dabbelt]

Nothing else in the kernel defines this, and this header is visible to
userspace.  Rather than hiding it in an #ifdef, I think it's sane to
just make this visible to userspace.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/fb.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
index fb795c3b3c17..8926f13bc19f 100644
--- a/include/uapi/linux/fb.h
+++ b/include/uapi/linux/fb.h
@@ -392,11 +392,8 @@ struct fb_cursor {
 	struct fb_image	image;	/* Cursor image */
 };
 
-#ifdef CONFIG_FB_BACKLIGHT
 /* Settings for the generic backlight code */
 #define FB_BACKLIGHT_LEVELS	128
 #define FB_BACKLIGHT_MAX	0xFF
-#endif
-
 
 #endif /* _UAPI_LINUX_FB_H */
-- 
2.4.6

[PATCH 08/13] Hide MAX_SHARED_LIBS behind #ifdef __KERNEL__

[Palmer Dabbelt]

I'm not sure what this is, but it doesn't feel like something that
should be exposed to userspace here.  I'm assuming this file was
exposed for the structure in it, which doesn't depend on
MAX_SHARED_LIBS.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/flat.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/flat.h b/include/uapi/linux/flat.h
index 88cd6baba8f3..4ed679f3591e 100644
--- a/include/uapi/linux/flat.h
+++ b/include/uapi/linux/flat.h
@@ -13,11 +13,13 @@
 
 #define	FLAT_VERSION			0x00000004L
 
+#ifdef __KERNEL__
 #ifdef CONFIG_BINFMT_SHARED_FLAT
 #define	MAX_SHARED_LIBS			(4)
 #else
 #define	MAX_SHARED_LIBS			(1)
 #endif
+#endif /*__KERNEL__*/
 
 /*
  * To make everything easier to port and manage cross platform
-- 
2.4.6

[PATCH 09/13] Hide bp_type_idx behind #ifdef __KERNEL__

[Palmer Dabbelt]

I'm actually not sure what to do here: if this enum is meant to be
used by userspace, then it has to be the same regardless of kernel
configuration.  One option would be to have the kernel expose all the
values to userspace and then map them internally if
CONFIG_HAVE_MIXED_BREAKPOINT_REGS isn't set, but that feels like it'd
be more invasive.

Here I took the simple and fail-fast route to hide all the
definitions.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/hw_breakpoint.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/hw_breakpoint.h b/include/uapi/linux/hw_breakpoint.h
index b04000a2296a..2498bfbf56c4 100644
--- a/include/uapi/linux/hw_breakpoint.h
+++ b/include/uapi/linux/hw_breakpoint.h
@@ -17,14 +17,16 @@ enum {
 	HW_BREAKPOINT_INVALID   = HW_BREAKPOINT_RW | HW_BREAKPOINT_X,
 };
 
+#ifdef __KERNEL__
 enum bp_type_idx {
 	TYPE_INST 	= 0,
-#ifdef CONFIG_HAVE_MIXED_BREAKPOINTS_REGS
+#if defined(CONFIG_HAVE_MIXED_BREAKPOINTS_REGS)
 	TYPE_DATA	= 0,
 #else
 	TYPE_DATA	= 1,
 #endif
 	TYPE_MAX
 };
+#endif /* __KERNEL__ */
 
 #endif /* _UAPI_LINUX_HW_BREAKPOINT_H */
-- 
2.4.6

[PATCH 10/13] Hide USE_WCACHING behind #ifdef __KERNEL__

[Palmer Dabbelt]

I don't think this was ever intended to be exposed to userspace.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/pktcdvd.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/pktcdvd.h b/include/uapi/linux/pktcdvd.h
index 2640b9d4e243..e2a49bfee74e 100644
--- a/include/uapi/linux/pktcdvd.h
+++ b/include/uapi/linux/pktcdvd.h
@@ -33,11 +33,13 @@
  * able to successfully recover with this option (drive will return good
  * status as soon as the cdb is validated).
  */
+#ifdef __KERNEL__
 #if defined(CONFIG_CDROM_PKTCDVD_WCACHE)
 #define USE_WCACHING		1
 #else
 #define USE_WCACHING		0
 #endif
+#endif /* __KERNEL__ */
 
 /*
  * No user-servicable parts beyond this point ->
-- 
2.4.6

[PATCH 11/13] Hide MAX_RAW_MINORS behind #ifdef __KERNEL__

[Palmer Dabbelt]

I don't think this was ever meant to be exposed to userspace.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/raw.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/raw.h b/include/uapi/linux/raw.h
index 62d543e70603..6d8d87524ff2 100644
--- a/include/uapi/linux/raw.h
+++ b/include/uapi/linux/raw.h
@@ -13,6 +13,8 @@ struct raw_config_request
 	__u64	block_minor;
 };
 
+#ifdef __KERNEL__
 #define MAX_RAW_MINORS CONFIG_MAX_RAW_DEVS
+#endif
 
 #endif /* __LINUX_RAW_H */
-- 
2.4.6

[PATCH 12/13] Hide AT_VECTOR_SIZE_ARCH behind #ifdef __KERNEL__

[Palmer Dabbelt]

I think this actually isn't a good idea, but I can't find anything
outside of the kernel that's using this so I'm going to hide it.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 arch/x86/include/uapi/asm/auxvec.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/include/uapi/asm/auxvec.h b/arch/x86/include/uapi/asm/auxvec.h
index 77203ac352de..88e5fa6d97e7 100644
--- a/arch/x86/include/uapi/asm/auxvec.h
+++ b/arch/x86/include/uapi/asm/auxvec.h
@@ -9,11 +9,13 @@
 #endif
 #define AT_SYSINFO_EHDR		33
 
+#ifdef __KERNEL__
 /* entries in ARCH_DLINFO: */
 #if defined(CONFIG_IA32_EMULATION) || !defined(CONFIG_X86_64)
 # define AT_VECTOR_SIZE_ARCH 2
 #else /* else it's non-compat x86-64 */
 # define AT_VECTOR_SIZE_ARCH 1
 #endif
+#endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_AUXVEC_H */
-- 
2.4.6

[PATCH 13/13] Re-enable and clean up "check_config()" in headers_check.pl

[Palmer Dabbelt]

I recently got bit by a CONFIG_ in userspace bug.  This has apparently
happened before, but the check got disabled for triggering too much.
In order to reduce false positives, I added some hueristics to avoid
detecting comments.

Since these tests all pass, I've now re-enabled them.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 scripts/headers_check.pl | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl
index 62320f93e903..dd413ad9c850 100755
--- a/scripts/headers_check.pl
+++ b/scripts/headers_check.pl
@@ -40,7 +40,7 @@ foreach my $file (@files) {
 		&check_asm_types();
 		&check_sizetypes();
 		&check_declarations();
-		# Dropped for now. Too much noise &check_config();
+		&check_config();
 	}
 	close $fh;
 }
@@ -76,9 +76,24 @@ sub check_declarations
 	}
 }
 
+my $check_config_in_multiline_comment = 0;
 sub check_config
 {
-	if ($line =~ m/[^a-zA-Z0-9_]+CONFIG_([a-zA-Z0-9_]+)[^a-zA-Z0-9_]/) {
+	my $nocomments = $line;
+	$nocomments =~ s/\/\*.*\*\///; # Remove ANSI-style comments (/* to */)
+	$nocomments =~ s/\/\/.*//;     # Remove C99-style comments (// to EOL)
+
+	# Check to see if we're within a multiline comment, and if so
+	# just remove the whole line.  I tried matching on '^ * ', but
+	# there's one header that doesn't prepend multi-line comments
+	# with * so that won't work.
+	if ($nocomments =~ m/\/\*/) { $check_config_in_multiline_comment = 1; }
+	if ($nocomments =~ m/\*\//) { $check_config_in_multiline_comment = 0; }
+	if ($check_config_in_multiline_comment = 1) { $nocomments = "" }
+
+	# Check to see if there is something that looks like CONFIG_
+	# inside a userspace-accessible header file and if so, print that out.
+	if ($nocomments =~ m/[^a-zA-Z0-9_]+CONFIG_([a-zA-Z0-9_]+)[^a-zA-Z0-9_]/) {
 		printf STDERR "$filename:$lineno: leaks CONFIG_$1 to userspace where it is not valid\n";
 	}
 }
-- 
2.4.6

Re: [PATCH 09/13] Hide bp_type_idx behind #ifdef __KERNEL__

[David Howells]

Palmer Dabbelt <palmer@dabbelt.com> wrote:

> +#ifdef __KERNEL__
>  enum bp_type_idx {
>  	TYPE_INST 	= 0,
> -#ifdef CONFIG_HAVE_MIXED_BREAKPOINTS_REGS
> +#if defined(CONFIG_HAVE_MIXED_BREAKPOINTS_REGS)
>  	TYPE_DATA	= 0,
>  #else
>  	TYPE_DATA	= 1,
>  #endif
>  	TYPE_MAX
>  };
> +#endif /* __KERNEL__ */

This should be in include/linux/hw_breakpoint.h without __KERNEL__ markings.

David

Re: [PATCH 10/13] Hide USE_WCACHING behind #ifdef __KERNEL__

[David Howells]

Palmer Dabbelt <palmer@dabbelt.com> wrote:

> I don't think this was ever intended to be exposed to userspace.

Then it should be in include/linux/pktcdvd.h

David

Re: [PATCH 11/13] Hide MAX_RAW_MINORS behind #ifdef __KERNEL__

[David Howells]

Palmer Dabbelt <palmer@dabbelt.com> wrote:

> I don't think this was ever meant to be exposed to userspace.

Then it should be in include/linux/raw.h

David

Re: [PATCH 12/13] Hide AT_VECTOR_SIZE_ARCH behind #ifdef __KERNEL__

[David Howells]

Palmer Dabbelt <palmer@dabbelt.com> wrote:

> I think this actually isn't a good idea, but I can't find anything
> outside of the kernel that's using this so I'm going to hide it.

It should be in arch/x86/include/asm/auxvec.h then.

Re: [PATCH] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h

[David Howells]

Rather than iterating through all the rest of your patches and saying the same
thing, if there's something in a UAPI header that needs wrapping in __KERNEL__
to exclude it from userspace's use, then it should be transferred to the
non-UAPI variant of that header (which should #include the UAPI variant).

David

Re: [PATCH] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h

[David Howells]

David Howells <dhowells@redhat.com> wrote:

> Rather than iterating through all the rest of your patches and saying the same
> thing, if there's something in a UAPI header that needs wrapping in __KERNEL__
> to exclude it from userspace's use, then it should be transferred to the
> non-UAPI variant of that header (which should #include the UAPI variant).

I should mention that there is the odd case where this is difficult to
achieve.  See include/uapi/linux/acct.h for an example...

David

Re: [PATCH] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h

[Palmer Dabbelt]

On Thu, 10 Sep 2015 04:18:05 PDT (-0700), dhowells@redhat.com wrote:
> David Howells <dhowells@redhat.com> wrote:
>
>> Rather than iterating through all the rest of your patches and saying the same
>> thing, if there's something in a UAPI header that needs wrapping in __KERNEL__
>> to exclude it from userspace's use, then it should be transferred to the
>> non-UAPI variant of that header (which should #include the UAPI variant).
>
> I should mention that there is the odd case where this is difficult to
> achieve.  See include/uapi/linux/acct.h for an example...

OK, sorry about that.  I'm submitting a v3 that should fix these
problems.