Patchwork [GIT,PULL,-tip] fix 33 make headers_check warnings

login
register
mail settings
Submitter Jaswinder Singh Rajput
Date Jan. 21, 2009, 12:38 a.m.
Message ID <1232498319.3123.34.camel@localhost.localdomain>
Download mbox | patch
Permalink /patch/19560/
State Accepted
Headers show

Comments

Jaswinder Singh Rajput - Jan. 21, 2009, 12:38 a.m.
On Sat, 2009-01-17 at 14:26 -0800, H. Peter Anvin wrote:
> Sam Ravnborg wrote:
> >>>  
> >> That patch looks wrong, and unnecessary. It was fine before.
> > Nope - include/linux/dvb/audio.h failed to include linux/types.h
> > despite the fact that is uses __u32 etc.
> > 
> > But why the _kernel_ should include a userspace header is
> > much more questionable.
> > 
> 
> <stdint.h> is one of a handful of headers provided by gcc itself.
> 

Should I reintroduce my patch to solve this warning of 'make headers_check':
 usr/include/linux/dvb/audio.h:133: found __[us]{8,16,32,64} type without #include <linux/types.h>

Or you guys have some better plan.

Thanks,

--
JSR
Jaswinder Singh Rajput - Jan. 23, 2009, 3:59 p.m.
On Wed, 2009-01-21 at 06:08 +0530, Jaswinder Singh Rajput wrote:
> On Sat, 2009-01-17 at 14:26 -0800, H. Peter Anvin wrote:
> > Sam Ravnborg wrote:
> > >>>  
> > >> That patch looks wrong, and unnecessary. It was fine before.
> > > Nope - include/linux/dvb/audio.h failed to include linux/types.h
> > > despite the fact that is uses __u32 etc.
> > > 
> > > But why the _kernel_ should include a userspace header is
> > > much more questionable.
> > > 
> > 
> > <stdint.h> is one of a handful of headers provided by gcc itself.
> > 
> 
> Should I reintroduce my patch to solve this warning of 'make headers_check':
>  usr/include/linux/dvb/audio.h:133: found __[us]{8,16,32,64} type without #include <linux/types.h>
> 
> diff --git a/include/linux/dvb/audio.h b/include/linux/dvb/audio.h
> index 89412e1..758a48c 100644
> --- a/include/linux/dvb/audio.h
> +++ b/include/linux/dvb/audio.h
> @@ -24,9 +24,8 @@
>  #ifndef _DVBAUDIO_H_
>  #define _DVBAUDIO_H_
>  
> -#ifdef __KERNEL__
>  #include <linux/types.h>
> -#else
> +#ifndef __KERNEL__
>  #include <stdint.h>
>  #endif
> 

It seems one have objection for this. So I will again insert this in my
new patchset.

--
JSR
Jaswinder Singh Rajput - Jan. 23, 2009, 4:04 p.m.
On Fri, 2009-01-23 at 21:29 +0530, Jaswinder Singh Rajput wrote:
> On Wed, 2009-01-21 at 06:08 +0530, Jaswinder Singh Rajput wrote:
> > On Sat, 2009-01-17 at 14:26 -0800, H. Peter Anvin wrote:
> > > Sam Ravnborg wrote:
> > > >>>  
> > > >> That patch looks wrong, and unnecessary. It was fine before.
> > > > Nope - include/linux/dvb/audio.h failed to include linux/types.h
> > > > despite the fact that is uses __u32 etc.
> > > > 
> > > > But why the _kernel_ should include a userspace header is
> > > > much more questionable.
> > > > 
> > > 
> > > <stdint.h> is one of a handful of headers provided by gcc itself.
> > > 
> > 
> > Should I reintroduce my patch to solve this warning of 'make headers_check':
> >  usr/include/linux/dvb/audio.h:133: found __[us]{8,16,32,64} type without #include <linux/types.h>
> > 
> > diff --git a/include/linux/dvb/audio.h b/include/linux/dvb/audio.h
> > index 89412e1..758a48c 100644
> > --- a/include/linux/dvb/audio.h
> > +++ b/include/linux/dvb/audio.h
> > @@ -24,9 +24,8 @@
> >  #ifndef _DVBAUDIO_H_
> >  #define _DVBAUDIO_H_
> >  
> > -#ifdef __KERNEL__
> >  #include <linux/types.h>
> > -#else
> > +#ifndef __KERNEL__
> >  #include <stdint.h>
> >  #endif
> > 
> 
> It seems one have objection for this. So I will again insert this in my
> new patchset.
> 

oops, s/seems one/seems no one ;-)

--
JSR
Jan Engelhardt - Jan. 23, 2009, 4:11 p.m.
On Friday 2009-01-23 17:04, Jaswinder Singh Rajput wrote:
>> > diff --git a/include/linux/dvb/audio.h b/include/linux/dvb/audio.h
>> > index 89412e1..758a48c 100644
>> > --- a/include/linux/dvb/audio.h
>> > +++ b/include/linux/dvb/audio.h
>> > @@ -24,9 +24,8 @@
>> >  #ifndef _DVBAUDIO_H_
>> >  #define _DVBAUDIO_H_
>> >  
>> > -#ifdef __KERNEL__
>> >  #include <linux/types.h>
>> > -#else
>> > +#ifndef __KERNEL__
>> >  #include <stdint.h>
>> >  #endif
>> > 
>> 
>> It seems one have objection for this. So I will again insert this in my
>> new patchset.
>> 
>
>oops, s/seems one/seems no one ;-)

I had an objection as previously stated -- namely that
<stdint.h> should be included to remain friendly to C++0x
programs which should use <cstdint> instead. Forcing
stdint.h is therefore not nice.
Sam Ravnborg - Jan. 23, 2009, 4:21 p.m.
On Fri, Jan 23, 2009 at 05:11:13PM +0100, Jan Engelhardt wrote:
> 
> On Friday 2009-01-23 17:04, Jaswinder Singh Rajput wrote:
> >> > diff --git a/include/linux/dvb/audio.h b/include/linux/dvb/audio.h
> >> > index 89412e1..758a48c 100644
> >> > --- a/include/linux/dvb/audio.h
> >> > +++ b/include/linux/dvb/audio.h
> >> > @@ -24,9 +24,8 @@
> >> >  #ifndef _DVBAUDIO_H_
> >> >  #define _DVBAUDIO_H_
> >> >  
> >> > -#ifdef __KERNEL__
> >> >  #include <linux/types.h>
> >> > -#else
> >> > +#ifndef __KERNEL__
> >> >  #include <stdint.h>
> >> >  #endif
> >> > 
> >> 
> >> It seems one have objection for this. So I will again insert this in my
> >> new patchset.
> >> 
> >
> >oops, s/seems one/seems no one ;-)
> 
> I had an objection as previously stated -- namely that
> <stdint.h> should be included to remain friendly to C++0x
> programs which should use <cstdint> instead. Forcing
> stdint.h is therefore not nice.

This is fully agreed.
Jaswinder - can I ask you to do this change (remove of the stdint.h include)
in a follow-up patch. It is two independent changes.
You original patch is fine as is.

Thanks,
	Sam
Jaswinder Singh Rajput - Jan. 23, 2009, 4:31 p.m.
On Fri, 2009-01-23 at 17:21 +0100, Sam Ravnborg wrote:
> On Fri, Jan 23, 2009 at 05:11:13PM +0100, Jan Engelhardt wrote:
> > 
> > On Friday 2009-01-23 17:04, Jaswinder Singh Rajput wrote:
> > >> > diff --git a/include/linux/dvb/audio.h b/include/linux/dvb/audio.h
> > >> > index 89412e1..758a48c 100644
> > >> > --- a/include/linux/dvb/audio.h
> > >> > +++ b/include/linux/dvb/audio.h
> > >> > @@ -24,9 +24,8 @@
> > >> >  #ifndef _DVBAUDIO_H_
> > >> >  #define _DVBAUDIO_H_
> > >> >  
> > >> > -#ifdef __KERNEL__
> > >> >  #include <linux/types.h>
> > >> > -#else
> > >> > +#ifndef __KERNEL__
> > >> >  #include <stdint.h>
> > >> >  #endif
> > >> > 
> > >> 
> > >> It seems one have objection for this. So I will again insert this in my
> > >> new patchset.
> > >> 
> > >
> > >oops, s/seems one/seems no one ;-)
> > 
> > I had an objection as previously stated -- namely that
> > <stdint.h> should be included to remain friendly to C++0x
> > programs which should use <cstdint> instead. Forcing
> > stdint.h is therefore not nice.
> 
> This is fully agreed.
> Jaswinder - can I ask you to do this change (remove of the stdint.h include)
> in a follow-up patch. It is two independent changes.
> You original patch is fine as is.

So is this OK:

-#ifdef __KERNEL__
 #include <linux/types.h>
-#else
-#include <stdint.h>
-#endif
-

--
JSR
Sam Ravnborg - Jan. 23, 2009, 5:09 p.m.
On Fri, Jan 23, 2009 at 10:01:44PM +0530, Jaswinder Singh Rajput wrote:
> On Fri, 2009-01-23 at 17:21 +0100, Sam Ravnborg wrote:
> > On Fri, Jan 23, 2009 at 05:11:13PM +0100, Jan Engelhardt wrote:
> > > 
> > > On Friday 2009-01-23 17:04, Jaswinder Singh Rajput wrote:
> > > >> > diff --git a/include/linux/dvb/audio.h b/include/linux/dvb/audio.h
> > > >> > index 89412e1..758a48c 100644
> > > >> > --- a/include/linux/dvb/audio.h
> > > >> > +++ b/include/linux/dvb/audio.h
> > > >> > @@ -24,9 +24,8 @@
> > > >> >  #ifndef _DVBAUDIO_H_
> > > >> >  #define _DVBAUDIO_H_
> > > >> >  
> > > >> > -#ifdef __KERNEL__
> > > >> >  #include <linux/types.h>
> > > >> > -#else
> > > >> > +#ifndef __KERNEL__
> > > >> >  #include <stdint.h>
> > > >> >  #endif
> > > >> > 
> > > >> 
> > > >> It seems one have objection for this. So I will again insert this in my
> > > >> new patchset.
> > > >> 
> > > >
> > > >oops, s/seems one/seems no one ;-)
> > > 
> > > I had an objection as previously stated -- namely that
> > > <stdint.h> should be included to remain friendly to C++0x
> > > programs which should use <cstdint> instead. Forcing
> > > stdint.h is therefore not nice.
> > 
> > This is fully agreed.
> > Jaswinder - can I ask you to do this change (remove of the stdint.h include)
> > in a follow-up patch. It is two independent changes.
> > You original patch is fine as is.
> 
> So is this OK:
> 
> -#ifdef __KERNEL__
>  #include <linux/types.h>
> -#else
> -#include <stdint.h>
> -#endif

That was what I expected the final change to look like.
If you want then combine it in one patch.

	Sam
Jaswinder Singh Rajput - Jan. 23, 2009, 5:28 p.m.
On Fri, 2009-01-23 at 18:09 +0100, Sam Ravnborg wrote:
> On Fri, Jan 23, 2009 at 10:01:44PM +0530, Jaswinder Singh Rajput wrote:

> > So is this OK:
> > 
> > -#ifdef __KERNEL__
> >  #include <linux/types.h>
> > -#else
> > -#include <stdint.h>
> > -#endif
> 
> That was what I expected the final change to look like.
> If you want then combine it in one patch.
> 

Ok, thanks.

--
JSR
H. Peter Anvin - Jan. 23, 2009, 9:33 p.m.
Jan Engelhardt wrote:
> 
> I had an objection as previously stated -- namely that
> <stdint.h> should be included to remain friendly to C++0x
> programs which should use <cstdint> instead. Forcing
> stdint.h is therefore not nice.
> 

FWIW, it's kind of pointless in that case; <cstdint> exports it into the 
std:: namespace rather than the root namespace, so using stdint types 
still don't work.

It again comes down to: for headers exported to userspace we *have* to 
use double-underscore types.

	-hpa
Jan Engelhardt - Jan. 23, 2009, 9:54 p.m.
On Friday 2009-01-23 22:33, H. Peter Anvin wrote:
>>
>> I had an objection as previously stated -- namely that
>> <stdint.h> should be included to remain friendly to C++0x
>> programs which should use <cstdint> instead. Forcing
>> stdint.h is therefore not nice.
>
> FWIW, it's kind of pointless in that case; <cstdint> exports it into the std::
> namespace rather than the root namespace, so using stdint types still don't
> work.

Hm, maybe g++ defaults to std? Because this works without me using
"using std;"

#include <cstdint>
int main(void)
{
	uint32_t x;
}
H. Peter Anvin - Jan. 23, 2009, 10:01 p.m.
Jan Engelhardt wrote:
> 
> Hm, maybe g++ defaults to std? Because this works without me using
> "using std;"
> 
> #include <cstdint>
> int main(void)
> {
> 	uint32_t x;
> }
> 

Sorry, this was a bit of a surprise... basically the g++ <cstdint> 
functionally does #include <stdint.h> then exports the symbols into the 
std namespace, so it puts them into both...

	-hpa

Patch

diff --git a/include/linux/dvb/audio.h b/include/linux/dvb/audio.h
index 89412e1..758a48c 100644
--- a/include/linux/dvb/audio.h
+++ b/include/linux/dvb/audio.h
@@ -24,9 +24,8 @@ 
 #ifndef _DVBAUDIO_H_
 #define _DVBAUDIO_H_
 
-#ifdef __KERNEL__
 #include <linux/types.h>
-#else
+#ifndef __KERNEL__
 #include <stdint.h>
 #endif