lkml.org 
[lkml]   [2009]   [Jan]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [GIT PULL -tip] fix 41 'make headers_check' warnings
From
Date
On Sun, 2009-01-18 at 12:02 +0100, Ingo Molnar wrote:
> * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
>
> > diff --git a/include/linux/acct.h b/include/linux/acct.h
> > index 882dc72..a20c97c 100644
> > --- a/include/linux/acct.h
> > +++ b/include/linux/acct.h
> > @@ -59,9 +59,13 @@ struct acct
> > comp_t ac_majflt; /* Major Pagefaults */
> > comp_t ac_swaps; /* Number of Swaps */
> > /* m68k had no padding here. */
> > -#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
> > +#ifdef __KERNEL__
> > +#ifndef CONFIG_M68K
> > __u16 ac_ahz; /* AHZ */
> > -#endif
> > +#endif /* CONFIG_M68K */
> > +#else /* __KERNEL__ */
> > + __u16 ac_ahz; /* AHZ */
> > +#endif /* __KERNEL__ */
>
> that looks rather ugly.
>
> Why not just flip it around to:
>
> #if !defined(__KERNEL__) || !defined(CONFIG_M68K)
>
> ? Does headers_check misinterpret that?
>

This will not many any difference:
then usr/include/linux/acct.h will look like:
#if !defined(__KERNEL__) || !defined(CONFIG_M68K)
__u16 ac_ahz; /* AHZ */
#endif
And we will get same warning:
usr/include/linux/acct.h:62: leaks CONFIG_M68K to userspace where it is not valid


> > * To make everything easier to port and manage cross platform
> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> > index 343df9e..1202063 100644
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> > @@ -528,6 +528,7 @@ struct sysinfo {
> > /* Trap pasters of __FUNCTION__ at compile-time */
> > #define __FUNCTION__ (__func__)
> >
> > +#ifdef __KERNEL__
> > /* This helps us to avoid #ifdef CONFIG_NUMA */
> > #ifdef CONFIG_NUMA
> > #define NUMA_BUILD 1
> > @@ -540,4 +541,8 @@ struct sysinfo {
> > # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
> > #endif
> >
> > +#else /* __KERNEL__ */
> > +#define NUMA_BUILD 0
> > +#endif /* __KERNEL__ */
>
> Does NUMA_BUILD make any sense to user-space at all? Shouldnt we leave it
> undefined?
>

OK, I can do this.

> > --- a/include/linux/pktcdvd.h
> > +++ b/include/linux/pktcdvd.h
> > @@ -33,11 +33,15 @@
> > * able to sucessfully 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
> > +#else /* __KERNEL__ */
> > +#define USE_WCACHING 0
> > +#endif /* __KERNEL__ */
>
> does USE_WCACHING make any sense to user-space? Shouldnt we leave it
> undefined?
>

Sure.

> > diff --git a/include/linux/raw.h b/include/linux/raw.h
> > index 62d543e..3898e30 100644
> > --- a/include/linux/raw.h
> > +++ b/include/linux/raw.h
> > @@ -13,6 +13,10 @@ struct raw_config_request
> > __u64 block_minor;
> > };
> >
> > +#ifdef __KERNEL__
> > #define MAX_RAW_MINORS CONFIG_MAX_RAW_DEVS
> > +#else /* __KERNEL__ */
> > +#define MAX_RAW_MINORS 0
> > +#endif /* __KERNEL__ */
>
> ditto.
>

OK.

> > #endif /* __LINUX_RAW_H */
> > diff --git a/include/linux/socket.h b/include/linux/socket.h
> > index f5771a2..d7daa52 100644
> > --- a/include/linux/socket.h
> > +++ b/include/linux/socket.h
> > @@ -256,11 +256,15 @@ struct ucred {
> > #define MSG_CMSG_CLOEXEC 0x40000000 /* Set close_on_exit for file
> > descriptor received through
> > SCM_RIGHTS */
> > +#ifdef __KERNEL__
> > #if defined(CONFIG_COMPAT)
> > #define MSG_CMSG_COMPAT 0x80000000 /* This message needs 32 bit fixups */
> > #else
> > #define MSG_CMSG_COMPAT 0 /* We never have 32 bit fixups */
> > #endif
> > +#else /* __KERNEL__ */
> > +#define MSG_CMSG_COMPAT 0 /* We never have 32 bit fixups */
> > +#endif /* __KERNEL__ */
>
> I suspect this flag should always be defined for user-space - the zero
> value only makes sense in the kernel.
>

OK.

> > --- a/include/linux/types.h
> > +++ b/include/linux/types.h
> > @@ -138,6 +138,7 @@ typedef __s64 int64_t;
> > *
> > * blkcnt_t is the type of the inode's block count.
> > */
> > +#ifdef __KERNEL__
> > #ifdef CONFIG_LBD
> > typedef u64 sector_t;
> > typedef u64 blkcnt_t;
> > @@ -145,6 +146,10 @@ typedef u64 blkcnt_t;
> > typedef unsigned long sector_t;
> > typedef unsigned long blkcnt_t;
> > #endif
> > +#else /* __KERNEL__ */
> > +typedef unsigned long sector_t;
> > +typedef unsigned long blkcnt_t;
> > +#endif /* __KERNEL__ */
>
> heh. types.h itself is not headers_check clean.
>
> But this isnt particularly clean: we have now 3 blocks of typedefs while
> there are just 2 variants. It would be cleaner to do something like:
>
> #if !defined(__KERNEL__) || defined(CONFIG_LBD)
>
> i.e. always provide the wider type to user-space.

Sorry, this will not be helpful we will still get the warning.

Now we need to decide, should we manage with 3 blocks or should we stay
with warnings.

But usr/include/types.h looks pretty clean:
/**
* The type used for indexing onto a disc or disc partition.
*
* Linux always considers sectors to be 512 bytes long independently
* of the devices real block size.
*
* blkcnt_t is the type of the inode's block count.
*/
typedef unsigned long sector_t;
typedef unsigned long blkcnt_t;
--
JSR



\
 
 \ /
  Last update: 2009-01-18 12:33    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site