tipc_config.h requires linux/string.h, which does not exist in exported headers

tipc_config.h requires linux/string.h, which does not exist in exported headers

[Alex Riesen]

Sorry for random Cc: list.

v2.6.24-rc1-423-g97855b4, with CONFIG_HEADERS_CHECK=y

$ make
...
  CHECK   include/linux/tipc_config.h
/home/raa/linux/usr/include/linux/tipc_config.h requires linux/string.h, which does not exist in exported headers
make[3]: *** [/home/raa/linux/usr/include/linux/.check.tipc_config.h] Error 1
make[2]: *** [linux] Error 2
make[1]: *** [headers_check] Error 2
make: *** [vmlinux] Error 2

Re: tipc_config.h requires linux/string.h, which does not exist in exported headers

[Sam Ravnborg]

On Tue, Oct 30, 2007 at 08:13:17PM +0100, Alex Riesen wrote:
> Sorry for random Cc: list.
> 
> v2.6.24-rc1-423-g97855b4, with CONFIG_HEADERS_CHECK=y
> 
> $ make
> ...
>   CHECK   include/linux/tipc_config.h
> /home/raa/linux/usr/include/linux/tipc_config.h requires linux/string.h, which does not exist in exported headers
> make[3]: *** [/home/raa/linux/usr/include/linux/.check.tipc_config.h] Error 1
> make[2]: *** [linux] Error 2
> make[1]: *** [headers_check] Error 2
> make: *** [vmlinux] Error 2

tipc_config.h uses memcpy in an inline function that is
never used in the kernel.
I'm awaiting response from tipc people if we can kill
that inline or we should make it a macro.
But davem decided just to include tipc_config.h and will now
sort it out.

	Sam

Re: tipc_config.h requires linux/string.h, which does not exist in exported headers

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Tue, 30 Oct 2007 22:54:09 +0100

> tipc_config.h uses memcpy in an inline function that is
> never used in the kernel.
> I'm awaiting response from tipc people if we can kill
> that inline or we should make it a macro.
> But davem decided just to include tipc_config.h and will now
> sort it out.

I think the thing to do is just __KERNEL__ protect the
<linux/string.h> include and require userspace to
include <string.h> itself when using these headers.

That's what I'm testing right now.

I'm pretty sure those inlines are indeed used by userspace.

Re: tipc_config.h requires linux/string.h, which does not exist in exported headers

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 30 Oct 2007 14:59:26 -0700 (PDT)

> I think the thing to do is just __KERNEL__ protect the
> <linux/string.h> include and require userspace to
> include <string.h> itself when using these headers.
> 
> That's what I'm testing right now.
> 
> I'm pretty sure those inlines are indeed used by userspace.

Actually, I'm tempted to put <string.h> in the ifndef __KERNEL__
block of that header.

That's ugly.

An alternative is to add linux/string.h to unifdef-y and have it
include <string.h> when !__KERNEL__ in order to handle cases like
this.

Sam, what do you think of that idea?

Re: tipc_config.h requires linux/string.h, which does not exist in exported headers

[Sam Ravnborg]

On Tue, Oct 30, 2007 at 03:05:37PM -0700, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 30 Oct 2007 14:59:26 -0700 (PDT)
> 
> > I think the thing to do is just __KERNEL__ protect the
> > <linux/string.h> include and require userspace to
> > include <string.h> itself when using these headers.
> > 
> > That's what I'm testing right now.
> > 
> > I'm pretty sure those inlines are indeed used by userspace.
> 
> Actually, I'm tempted to put <string.h> in the ifndef __KERNEL__
> block of that header.

Something like this..
I like it - much better than adding string.h to unidef-y.
PS - had not pulled latest -linus - so you must update Kbuild.
PPS - tipc.h should be exported too as I understood Stephen.

	Sam

diff --git a/include/linux/tipc_config.h b/include/linux/tipc_config.h
index b0c916d..30ddf1e 100644
--- a/include/linux/tipc_config.h
+++ b/include/linux/tipc_config.h
@@ -38,7 +38,6 @@
 #define _LINUX_TIPC_CONFIG_H_
 
 #include <linux/types.h>
-#include <linux/string.h>
 #include <asm/byteorder.h>
 
 /*
@@ -390,7 +389,8 @@ struct tipc_cfg_msg_hdr
 #define TCM_LENGTH(datalen) (sizeof(struct tipc_cfg_msg_hdr) + datalen)
 #define TCM_SPACE(datalen)  (TCM_ALIGN(TCM_LENGTH(datalen)))
 #define TCM_DATA(tcm_hdr)   ((void *)((char *)(tcm_hdr) + TCM_LENGTH(0)))
-
+#ifndef __KERNEL__
+#include <string.h>
 static inline int TCM_SET(void *msg, __u16 cmd, __u16 flags,
 			  void *data, __u16 data_len)
 {
@@ -406,5 +406,5 @@ static inline int TCM_SET(void *msg, __u16 cmd, __u16 flags,
 		memcpy(TCM_DATA(msg), data, data_len);
 	return TCM_SPACE(data_len);
 }
-
+#endif /* ifndef __KERNEL__ */
 #endif

Re: tipc_config.h requires linux/string.h, which does not exist in exported headers

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Tue, 30 Oct 2007 23:20:25 +0100

> On Tue, Oct 30, 2007 at 03:05:37PM -0700, David Miller wrote:
> > From: David Miller <davem@davemloft.net>
> > Date: Tue, 30 Oct 2007 14:59:26 -0700 (PDT)
> > 
> > > I think the thing to do is just __KERNEL__ protect the
> > > <linux/string.h> include and require userspace to
> > > include <string.h> itself when using these headers.
> > > 
> > > That's what I'm testing right now.
> > > 
> > > I'm pretty sure those inlines are indeed used by userspace.
> > 
> > Actually, I'm tempted to put <string.h> in the ifndef __KERNEL__
> > block of that header.
> 
> Something like this..
> I like it - much better than adding string.h to unidef-y.
> PS - had not pulled latest -linus - so you must update Kbuild.
> PPS - tipc.h should be exported too as I understood Stephen.

tipc.h is already listed in header-y.

I'll merge your patch, thanks Sam.

Re: tipc_config.h requires linux/string.h, which does not exist in exported headers

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 30 Oct 2007 15:31:54 -0700 (PDT)

> From: Sam Ravnborg <sam@ravnborg.org>
> Date: Tue, 30 Oct 2007 23:20:25 +0100
> 
> > Something like this..
> > I like it - much better than adding string.h to unidef-y.
> > PS - had not pulled latest -linus - so you must update Kbuild.
> > PPS - tipc.h should be exported too as I understood Stephen.
> 
> tipc.h is already listed in header-y.
> 
> I'll merge your patch, thanks Sam.

Unfortunately I have to back it out, it breaks the build.

In file included from net/tipc/core.h:41,
                 from net/tipc/addr.c:37:
include/linux/tipc_config.h: In function 'TLV_SET':
include/linux/tipc_config.h:306: error: implicit declaration of function 'memcpy'
include/linux/tipc_config.h:306: warning: incompatible implicit declaration of built-in function 'memcpy'

I truly think adding linux/string.h to unifdef-y along with:

#ifndef __KERNEL__
#include <string.h>
#else
 ...
#endif

in linux/string.h is a much cleaner and less error prone solution :-)

Re: tipc_config.h requires linux/string.h, which does not exist in exported headers

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 30 Oct 2007 21:14:04 -0700 (PDT)

> Unfortunately I have to back it out, it breaks the build.
> 
> In file included from net/tipc/core.h:41,
>                  from net/tipc/addr.c:37:
> include/linux/tipc_config.h: In function 'TLV_SET':
> include/linux/tipc_config.h:306: error: implicit declaration of function 'memcpy'
> include/linux/tipc_config.h:306: warning: incompatible implicit declaration of built-in function 'memcpy'
> 
> I truly think adding linux/string.h to unifdef-y along with:
> 
> #ifndef __KERNEL__
> #include <string.h>
> #else
>  ...
> #endif
> 
> in linux/string.h is a much cleaner and less error prone solution :-)

Here is what I mean, specifically.  And this is build tested :-)

>From 97ef1bb0c8e371b7988287f38bd107c4aa14d78d Mon Sep 17 00:00:00 2001
From: David S. Miller <davem@sunset.davemloft.net>
Date: Tue, 30 Oct 2007 21:44:00 -0700
Subject: [PATCH] [TIPC]: Fix headercheck wrt. tipc_config.h

It wants string functions like memcpy() for inline
routines, and these define userland interfaces.

The only clean way to deal with this is to simply
put linux/string.h into unifdef-y and have it
include <string.h> when not-__KERNEL__.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/linux/Kbuild   |    1 +
 include/linux/string.h |   12 +++---------
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/include/linux/Kbuild b/include/linux/Kbuild
index bd33c22..37bfa19 100644
--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -326,6 +326,7 @@ unifdef-y += sonypi.h
 unifdef-y += soundcard.h
 unifdef-y += stat.h
 unifdef-y += stddef.h
+unifdef-y += string.h
 unifdef-y += synclink.h
 unifdef-y += sysctl.h
 unifdef-y += tcp.h
diff --git a/include/linux/string.h b/include/linux/string.h
index 836062b..c5d3fca 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -3,16 +3,14 @@
 
 /* We don't want strings.h stuff being user by user stuff by accident */
 
-#ifdef __KERNEL__
+#ifndef __KERNEL__
+#include <string.h>
+#else
 
 #include <linux/compiler.h>	/* for inline */
 #include <linux/types.h>	/* for size_t */
 #include <linux/stddef.h>	/* for NULL */
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 extern char *strndup_user(const char __user *, long);
 
 /*
@@ -111,9 +109,5 @@ extern void *kmemdup(const void *src, size_t len, gfp_t gfp);
 extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
 extern void argv_free(char **argv);
 
-#ifdef __cplusplus
-}
-#endif
-
 #endif
 #endif /* _LINUX_STRING_H_ */
-- 
1.5.2.5

Re: tipc_config.h requires linux/string.h, which does not exist in exported headers

[Alex Riesen]

David Miller, Wed, Oct 31, 2007 05:56:16 +0100:
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 836062b..c5d3fca 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -3,16 +3,14 @@
>  
>  /* We don't want strings.h stuff being user by user stuff by accident */
>  

While at it, could you fix this typo: "being use_d_ by user stuff by accident"?

Re: tipc_config.h requires linux/string.h, which does not exist in exported headers

[Sam Ravnborg]

> 
> Here is what I mean, specifically.  And this is build tested :-)
> 
> From 97ef1bb0c8e371b7988287f38bd107c4aa14d78d Mon Sep 17 00:00:00 2001
> From: David S. Miller <davem@sunset.davemloft.net>
> Date: Tue, 30 Oct 2007 21:44:00 -0700
> Subject: [PATCH] [TIPC]: Fix headercheck wrt. tipc_config.h
> 
> It wants string functions like memcpy() for inline
> routines, and these define userland interfaces.
> 
> The only clean way to deal with this is to simply
> put linux/string.h into unifdef-y and have it
> include <string.h> when not-__KERNEL__.

Hi David.

Thanks for fixing this.
I will close the bug when the correcting hit mainline.

	Sam