| Message ID | 1232498319.3123.34.camel@localhost.localdomain |
|---|---|
| State | Accepted |
| Headers | show |
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
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
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.
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
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
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
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
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
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; }
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
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