public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL -tip] fix 41 'make headers_check' warnings
@ 2009-01-18 10:10 Jaswinder Singh Rajput
  2009-01-18 11:02 ` Ingo Molnar
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Jaswinder Singh Rajput @ 2009-01-18 10:10 UTC (permalink / raw)
  To: Ingo Molnar, Andrew Morton, Sam Ravnborg, x86 maintainers, LKML


The following changes since commit 2a927058df624d1c19bd42a90cb91ae148f12651:
  Ingo Molnar (1):
        Merge branch 'core/header-fixes'

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/linux-2.6-tipclean.git master

Jaswinder Singh Rajput (16):
      headers_check fix: linux/acct.h
      headers_check fix: linux/atmdev.h
      headers_check fix: linux/cm4000_cs.h
      headers_check fix: linux/elf-fdpic.h
      headers_check fix: linux/elfcore.h
      headers_check fix: linux/fb.h
      headers_check fix: linux/flat.h
      headers_check fix: linux/kernel.h
      headers_check fix: linux/kvm.h
      headers_check fix: linux/pkt_cls.h
      headers_check fix: linux/pktcdvd.h
      headers_check fix: linux/raw.h
      headers_check fix: linux/socket.h
      headers_check fix: linux/sound.h
      headers_check fix: linux/types.h
      headers_check fix: linux/videodev2.h

 include/linux/acct.h      |    8 ++++++--
 include/linux/atmdev.h    |    2 ++
 include/linux/cm4000_cs.h |    2 +-
 include/linux/elf-fdpic.h |    2 ++
 include/linux/elfcore.h   |    2 ++
 include/linux/fb.h        |    4 ++--
 include/linux/flat.h      |    8 +++++---
 include/linux/kernel.h    |    5 +++++
 include/linux/kvm.h       |    8 ++++++++
 include/linux/pkt_cls.h   |    4 ++--
 include/linux/pktcdvd.h   |    4 ++++
 include/linux/raw.h       |    4 ++++
 include/linux/socket.h    |    4 ++++
 include/linux/sound.h     |    2 ++
 include/linux/types.h     |    5 +++++
 include/linux/videodev2.h |    2 +-
 16 files changed, 55 insertions(+), 11 deletions(-)

This patchset fix the following 'make headers_check' warnings:

/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/acct.h:62: leaks CONFIG_M68K to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/atmdev.h:103: leaks CONFIG_COMPAT to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/cm4000_cs.h:22: leaks CONFIG_COMPAT to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/elf-fdpic.h:61: leaks CONFIG_MMU to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/elf-fdpic.h:62: extern's make no sense in userspace
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/elfcore.h:59: leaks CONFIG_BINFMT to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/fb.h:4: include of <linux/types.h> is preferred over <asm/types.h>
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/fb.h:152: found __[us]{8,16,32,64} type without #include <linux/types.h>
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/fb.h:381: leaks CONFIG_FB to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/flat.h:16: leaks CONFIG_BINFMT to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/kernel.h:39: leaks CONFIG_NUMA to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/kernel.h:40: leaks CONFIG_NUMA to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/kernel.h:46: leaks CONFIG_FTRACE to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/kernel.h:47: leaks CONFIG_FTRACE to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/kvm.h:10: include of <linux/types.h> is preferred over <asm/types.h>
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/kvm.h:19: found __[us]{8,16,32,64} type without #include <linux/types.h>
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/kvm.h:61: leaks CONFIG_X86 to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/kvm.h:64: leaks CONFIG_X86 to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/kvm.h:387: leaks CONFIG_X86 to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/kvm.h:391: leaks CONFIG_X86 to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/kvm.h:396: leaks CONFIG_X86 to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/pkt_cls.h:122: found __[us]{8,16,32,64} type without #include <linux/types.h>
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/pkt_cls.h:306: leaks CONFIG_NET to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/pkt_cls.h:307: leaks CONFIG_NET to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/pktcdvd.h:36: leaks CONFIG_CDROM to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/ppp_defs.h:50: found __[us]{8,16,32,64} type without #include <linux/types.h>
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/raw.h:16: leaks CONFIG_MAX to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/socket.h:27: leaks CONFIG_PROC to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/socket.h:29: extern's make no sense in userspace
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/socket.h:262: leaks CONFIG_COMPAT to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/sound.h:33: extern's make no sense in userspace
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/sound.h:34: extern's make no sense in userspace
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/sound.h:35: extern's make no sense in userspace
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/sound.h:36: extern's make no sense in userspace
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/sound.h:37: extern's make no sense in userspace
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/sound.h:39: extern's make no sense in userspace
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/sound.h:40: extern's make no sense in userspace
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/sound.h:41: extern's make no sense in userspace
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/sound.h:42: extern's make no sense in userspace
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/types.h:114: leaks CONFIG_LBD to userspace where it is not valid
/home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/videodev2.h:1477: leaks CONFIG_VIDEO to userspace where it is not valid

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__ */
 	__u32		ac_exitcode;		/* Exitcode */
 	char		ac_comm[ACCT_COMM + 1];	/* Command Name */
 	__u8		ac_etime_hi;		/* Elapsed Time MSB */
diff --git a/include/linux/atmdev.h b/include/linux/atmdev.h
index 086e5c3..ec81c12 100644
--- a/include/linux/atmdev.h
+++ b/include/linux/atmdev.h
@@ -100,10 +100,12 @@ 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 __KERNEL__
 #ifdef 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
+#endif /* __KERNEL__ */
 #define ATM_DROPPARTY 	_IOW('a', ATMIOC_SPECIAL+5,int)
 					/* drop party from p2mp call */
 
diff --git a/include/linux/cm4000_cs.h b/include/linux/cm4000_cs.h
index 605ebe2..cc8b2d9 100644
--- a/include/linux/cm4000_cs.h
+++ b/include/linux/cm4000_cs.h
@@ -19,7 +19,7 @@ typedef struct atreq {
 
 
 /* what is particularly stupid in the original driver is the arch-dependant
- * member sizes. This leads to CONFIG_COMPAT breakage, since 32bit userspace
+ * member sizes. This leads to COMPAT breakage, since 32bit userspace
  * will lay out the structure members differently than the 64bit kernel.
  *
  * I've changed "ptsreq.protocol" from "unsigned long" to "u_int32_t".
diff --git a/include/linux/elf-fdpic.h b/include/linux/elf-fdpic.h
index 9f5b745..7cd2e80 100644
--- a/include/linux/elf-fdpic.h
+++ b/include/linux/elf-fdpic.h
@@ -58,11 +58,13 @@ struct elf_fdpic_params {
 #define ELF_FDPIC_FLAG_PRESENT		0x80000000	/* T if this object is present */
 };
 
+#ifdef __KERNEL__
 #ifdef CONFIG_MMU
 extern void elf_fdpic_arch_lay_out_mm(struct elf_fdpic_params *exec_params,
 				      struct elf_fdpic_params *interp_params,
 				      unsigned long *start_stack,
 				      unsigned long *start_brk);
 #endif
+#endif /* __KERNEL__ */
 
 #endif /* _LINUX_ELF_FDPIC_H */
diff --git a/include/linux/elfcore.h b/include/linux/elfcore.h
index 5ca54d7..69f486d 100644
--- a/include/linux/elfcore.h
+++ b/include/linux/elfcore.h
@@ -64,6 +64,7 @@ struct elf_prstatus
 	long	pr_instr;		/* Current instruction */
 #endif
 	elf_gregset_t pr_reg;	/* GP registers */
+#ifdef __KERNEL__
 #ifdef 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
@@ -74,6 +75,7 @@ struct elf_prstatus
 	unsigned long pr_exec_fdpic_loadmap;
 	unsigned long pr_interp_fdpic_loadmap;
 #endif
+#endif /* __KERNEL__ */
 	int pr_fpvalid;		/* True if math co-processor being used.  */
 };
 
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 818fe21..abab732 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -382,14 +382,14 @@ struct fb_cursor {
 	struct fb_image	image;	/* Cursor image */
 };
 
+#ifdef __KERNEL__
+
 #ifdef CONFIG_FB_BACKLIGHT
 /* Settings for the generic backlight code */
 #define FB_BACKLIGHT_LEVELS	128
 #define FB_BACKLIGHT_MAX	0xFF
 #endif
 
-#ifdef __KERNEL__
-
 #include <linux/fs.h>
 #include <linux/init.h>
 #include <linux/device.h>
diff --git a/include/linux/flat.h b/include/linux/flat.h
index ec56852..7a1f69b 100644
--- a/include/linux/flat.h
+++ b/include/linux/flat.h
@@ -10,17 +10,19 @@
 #ifndef _LINUX_FLAT_H
 #define _LINUX_FLAT_H
 
+#define	FLAT_VERSION			0x00000004L
+
 #ifdef __KERNEL__
 #include <asm/flat.h>
-#endif
-
-#define	FLAT_VERSION			0x00000004L
 
 #ifdef CONFIG_BINFMT_SHARED_FLAT
 #define	MAX_SHARED_LIBS			(4)
 #else
 #define	MAX_SHARED_LIBS			(1)
 #endif
+#else /* __KERNEL__ */
+#define	MAX_SHARED_LIBS			(1)
+#endif /* __KERNEL__ */
 
 /*
  * 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__ */
+
 #endif
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 5715f19..5d004bc 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -58,12 +58,14 @@ struct kvm_irqchip {
 	__u32 pad;
         union {
 		char dummy[512];  /* reserving space */
+#ifdef __KERNEL__
 #ifdef CONFIG_X86
 		struct kvm_pic_state pic;
 #endif
 #if defined(CONFIG_X86) || defined(CONFIG_IA64)
 		struct kvm_ioapic_state ioapic;
 #endif
+#endif /* __KERNEL__ */
 	} chip;
 };
 
@@ -384,18 +386,24 @@ struct kvm_trace_rec {
 #define KVM_CAP_MP_STATE 14
 #define KVM_CAP_COALESCED_MMIO 15
 #define KVM_CAP_SYNC_MMU 16  /* Changes to host mmap are reflected in guest */
+#ifdef __KERNEL__
 #if defined(CONFIG_X86)||defined(CONFIG_IA64)
 #define KVM_CAP_DEVICE_ASSIGNMENT 17
 #endif
+#endif /* __KERNEL__ */
 #define KVM_CAP_IOMMU 18
+#ifdef __KERNEL__
 #if defined(CONFIG_X86)
 #define KVM_CAP_DEVICE_MSI 20
 #endif
+#endif /* __KERNEL__ */
 /* Bug in KVM_SET_USER_MEMORY_REGION fixed: */
 #define KVM_CAP_DESTROY_MEMORY_REGION_WORKS 21
+#ifdef __KERNEL__
 #if defined(CONFIG_X86)
 #define KVM_CAP_USER_NMI 22
 #endif
+#endif /* __KERNEL__ */
 
 /*
  * ioctls for VM fds
diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
index 3c842ed..7a45e09 100644
--- a/include/linux/pkt_cls.h
+++ b/include/linux/pkt_cls.h
@@ -304,8 +304,8 @@ enum
 	TCA_FW_UNSPEC,
 	TCA_FW_CLASSID,
 	TCA_FW_POLICE,
-	TCA_FW_INDEV, /*  used by CONFIG_NET_CLS_IND */
-	TCA_FW_ACT, /* used by CONFIG_NET_CLS_ACT */
+	TCA_FW_INDEV,	/* used by NET_CLS_IND */
+	TCA_FW_ACT,	/* used by NET_CLS_ACT */
 	TCA_FW_MASK,
 	__TCA_FW_MAX
 };
diff --git a/include/linux/pktcdvd.h b/include/linux/pktcdvd.h
index 04b4d73..277de8c 100644
--- 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__ */
 
 /*
  * No user-servicable parts beyond this point ->
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__ */
 
 #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__ */
 
 
 /* Setsockoptions(2) level. Thanks to BSD these must match IPPROTO_xxx */
diff --git a/include/linux/sound.h b/include/linux/sound.h
index 9e2a94f..44dcf05 100644
--- a/include/linux/sound.h
+++ b/include/linux/sound.h
@@ -25,6 +25,7 @@
 #define SND_DEV_AMIDI		13	/* Like /dev/midi (obsolete) */
 #define SND_DEV_ADMMIDI		14	/* Like /dev/dmmidi (onsolete) */
 
+#ifdef __KERNEL__
 /*
  *	Sound core interface functions
  */
@@ -40,3 +41,4 @@ extern void unregister_sound_special(int unit);
 extern void unregister_sound_mixer(int unit);
 extern void unregister_sound_midi(int unit);
 extern void unregister_sound_dsp(int unit);
+#endif /* __KERNEL__ */
diff --git a/include/linux/types.h b/include/linux/types.h
index 712ca53..6fa902a 100644
--- 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__ */
 
 /*
  * The type of an index into the pagecache.  Use a #define so asm/types.h
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 5571dbe..81fa255 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1480,7 +1480,7 @@ struct v4l2_chip_ident_old {
 
 #if 1
 /* Experimental, meant for debugging, testing and internal use.
-   Only implemented if CONFIG_VIDEO_ADV_DEBUG is defined.
+   Only implemented if VIDEO_ADV_DEBUG is defined.
    You must be root to use these ioctls. Never use these in applications! */
 #define	VIDIOC_DBG_S_REGISTER 	 _IOW('V', 79, struct v4l2_dbg_register)
 #define	VIDIOC_DBG_G_REGISTER 	_IOWR('V', 80, struct v4l2_dbg_register)



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 10:10 [GIT PULL -tip] fix 41 'make headers_check' warnings Jaswinder Singh Rajput
@ 2009-01-18 11:02 ` Ingo Molnar
  2009-01-18 11:26   ` Jaswinder Singh Rajput
                     ` (2 more replies)
  2009-01-18 22:39 ` [GIT PULL -tip] fix 41 'make headers_check' warnings Stephen Rothwell
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 22+ messages in thread
From: Ingo Molnar @ 2009-01-18 11:02 UTC (permalink / raw)
  To: Jaswinder Singh Rajput; +Cc: Andrew Morton, Sam Ravnborg, x86 maintainers, LKML


* 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?

>   * 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?

> --- 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?

> 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.

>  #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.

> --- 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.

	ngo

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 11:02 ` Ingo Molnar
@ 2009-01-18 11:26   ` Jaswinder Singh Rajput
  2009-01-18 12:26     ` Ingo Molnar
  2009-01-18 12:57   ` Sam Ravnborg
  2009-01-21  1:27   ` Jaswinder Singh Rajput
  2 siblings, 1 reply; 22+ messages in thread
From: Jaswinder Singh Rajput @ 2009-01-18 11:26 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Andrew Morton, Sam Ravnborg, x86 maintainers, LKML

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 11:26   ` Jaswinder Singh Rajput
@ 2009-01-18 12:26     ` Ingo Molnar
  0 siblings, 0 replies; 22+ messages in thread
From: Ingo Molnar @ 2009-01-18 12:26 UTC (permalink / raw)
  To: Jaswinder Singh Rajput; +Cc: Andrew Morton, Sam Ravnborg, x86 maintainers, LKML


* Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:

> 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

which would be a false positive. Sam, what would be your preference to 
annotate the code in such cases?

> > i.e. always provide the wider type to user-space.
> 
> Sorry, this will not be helpful we will still get the warning.

the problem here is that the type you export to user-space is the _wrong_ 
(narrow) type. Including types.h is one thing - exporting something to 
user-space is another thing. It might not matter in practice.

Furthermore, the 3 blocks are there as well - while there should only be 
two.

	Ingo

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 11:02 ` Ingo Molnar
  2009-01-18 11:26   ` Jaswinder Singh Rajput
@ 2009-01-18 12:57   ` Sam Ravnborg
  2009-01-18 13:00     ` Sam Ravnborg
  2009-01-21  1:27   ` Jaswinder Singh Rajput
  2 siblings, 1 reply; 22+ messages in thread
From: Sam Ravnborg @ 2009-01-18 12:57 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Jaswinder Singh Rajput, Andrew Morton, x86 maintainers, LKML

On Sun, Jan 18, 2009 at 12:02:21PM +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?

The original expression is misinterpreted by headers_check
because we want the ac_ahz to stay if either of __KERNEL__
or CONFIG_M68K is not defined.
And unifdef does not optimize away the !defined(CONFIG_M68K)
part - it has no knowledge that this is kernel internal.

So I am happy with Jaswinder's patch.

That said I really no not understand why there is this
subtle issue with struct acct in the first place..

	Sam

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 12:57   ` Sam Ravnborg
@ 2009-01-18 13:00     ` Sam Ravnborg
  2009-01-18 17:29       ` Ingo Molnar
  0 siblings, 1 reply; 22+ messages in thread
From: Sam Ravnborg @ 2009-01-18 13:00 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Jaswinder Singh Rajput, Andrew Morton, x86 maintainers, LKML

On Sun, Jan 18, 2009 at 01:57:22PM +0100, Sam Ravnborg wrote:
> On Sun, Jan 18, 2009 at 12:02:21PM +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?
> 
> The original expression is misinterpreted by headers_check
> because we want the ac_ahz to stay if either of __KERNEL__
> or CONFIG_M68K is not defined.
> And unifdef does not optimize away the !defined(CONFIG_M68K)
> part - it has no knowledge that this is kernel internal.
> 
> So I am happy with Jaswinder's patch.
Almost happy.
I digged out my original patch and it looks like this:
-#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
+#ifndef __KERNEL__
        __u16           ac_ahz;                 /* AHZ */
+#else
+  #ifndef CONFIG_M68K
+       __u16           ac_ahz;                 /* AHZ */
+  #endif
 #endif

The indention can be discussed..
But the logic is simpler.

	Sam

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 13:00     ` Sam Ravnborg
@ 2009-01-18 17:29       ` Ingo Molnar
  2009-01-21  0:58         ` Jaswinder Singh Rajput
  0 siblings, 1 reply; 22+ messages in thread
From: Ingo Molnar @ 2009-01-18 17:29 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: Jaswinder Singh Rajput, Andrew Morton, x86 maintainers, LKML


* Sam Ravnborg <sam@ravnborg.org> wrote:

> On Sun, Jan 18, 2009 at 01:57:22PM +0100, Sam Ravnborg wrote:
> > On Sun, Jan 18, 2009 at 12:02:21PM +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?
> > 
> > The original expression is misinterpreted by headers_check
> > because we want the ac_ahz to stay if either of __KERNEL__
> > or CONFIG_M68K is not defined.
> > And unifdef does not optimize away the !defined(CONFIG_M68K)
> > part - it has no knowledge that this is kernel internal.
> > 
> > So I am happy with Jaswinder's patch.
> Almost happy.
> I digged out my original patch and it looks like this:
> -#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
> +#ifndef __KERNEL__
>         __u16           ac_ahz;                 /* AHZ */
> +#else
> +  #ifndef CONFIG_M68K
> +       __u16           ac_ahz;                 /* AHZ */
> +  #endif
>  #endif
> 
> The indention can be discussed..
> But the logic is simpler.

yes - that's the 3-block versus 2-block issue i mentioned to Jaswinder. 
(the first version duplicated the same thing into 3 places - while the 
logic only splits it in two)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 10:10 [GIT PULL -tip] fix 41 'make headers_check' warnings Jaswinder Singh Rajput
  2009-01-18 11:02 ` Ingo Molnar
@ 2009-01-18 22:39 ` Stephen Rothwell
  2009-01-19  1:20   ` Sam Ravnborg
  2009-01-18 22:50 ` Stephen Rothwell
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Stephen Rothwell @ 2009-01-18 22:39 UTC (permalink / raw)
  To: Jaswinder Singh Rajput
  Cc: Ingo Molnar, Andrew Morton, Sam Ravnborg, x86 maintainers, LKML

[-- Attachment #1: Type: text/plain, Size: 812 bytes --]

Hi Jaswinder,

On Sun, 18 Jan 2009 15:40:54 +0530 Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
>
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 5571dbe..81fa255 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1480,7 +1480,7 @@ struct v4l2_chip_ident_old {
>  
>  #if 1
>  /* Experimental, meant for debugging, testing and internal use.
> -   Only implemented if CONFIG_VIDEO_ADV_DEBUG is defined.
> +   Only implemented if VIDEO_ADV_DEBUG is defined.
>     You must be root to use these ioctls. Never use these in applications! */

Is there some reason we cannot fix the tool that does the checking for
this case?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 10:10 [GIT PULL -tip] fix 41 'make headers_check' warnings Jaswinder Singh Rajput
  2009-01-18 11:02 ` Ingo Molnar
  2009-01-18 22:39 ` [GIT PULL -tip] fix 41 'make headers_check' warnings Stephen Rothwell
@ 2009-01-18 22:50 ` Stephen Rothwell
  2009-01-19  2:30   ` Jaswinder Singh Rajput
  2009-01-18 23:53 ` Stephen Rothwell
  2009-01-18 23:59 ` Stephen Rothwell
  4 siblings, 1 reply; 22+ messages in thread
From: Stephen Rothwell @ 2009-01-18 22:50 UTC (permalink / raw)
  To: Jaswinder Singh Rajput
  Cc: Ingo Molnar, Andrew Morton, Sam Ravnborg, x86 maintainers, LKML

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

Hi Jaswinder,

On Sun, 18 Jan 2009 15:40:54 +0530 Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
>
> 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__ */
>  
>  #endif /* __LINUX_RAW_H */

Since MAX_RAW_MINORS has only one user (drivers/char/raw.c), why not
just remove the define from here and add it it there?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 10:10 [GIT PULL -tip] fix 41 'make headers_check' warnings Jaswinder Singh Rajput
                   ` (2 preceding siblings ...)
  2009-01-18 22:50 ` Stephen Rothwell
@ 2009-01-18 23:53 ` Stephen Rothwell
  2009-01-19  2:31   ` Jaswinder Singh Rajput
  2009-01-18 23:59 ` Stephen Rothwell
  4 siblings, 1 reply; 22+ messages in thread
From: Stephen Rothwell @ 2009-01-18 23:53 UTC (permalink / raw)
  To: Jaswinder Singh Rajput
  Cc: Ingo Molnar, Andrew Morton, Sam Ravnborg, x86 maintainers, LKML

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

Hi Jaswinder,

On Sun, 18 Jan 2009 15:40:54 +0530 Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
>
> diff --git a/include/linux/pktcdvd.h b/include/linux/pktcdvd.h
> index 04b4d73..277de8c 100644
> --- 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__ */

This also only has one user (drivers/block/pktcdvd.c) so maybe it should
be moved there.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 10:10 [GIT PULL -tip] fix 41 'make headers_check' warnings Jaswinder Singh Rajput
                   ` (3 preceding siblings ...)
  2009-01-18 23:53 ` Stephen Rothwell
@ 2009-01-18 23:59 ` Stephen Rothwell
  4 siblings, 0 replies; 22+ messages in thread
From: Stephen Rothwell @ 2009-01-18 23:59 UTC (permalink / raw)
  To: Jaswinder Singh Rajput
  Cc: Ingo Molnar, Andrew Morton, Sam Ravnborg, x86 maintainers, LKML

[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]

Hi Jaswinder,

On Sun, 18 Jan 2009 15:40:54 +0530 Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
>
> 
> /home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/pkt_cls.h:306: leaks CONFIG_NET to userspace where it is not valid
> /home/jaswinder/jaswinder-git/linux-2.6-tip/usr/include/linux/pkt_cls.h:307: leaks CONFIG_NET to userspace where it is not valid

> diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
> index 3c842ed..7a45e09 100644
> --- a/include/linux/pkt_cls.h
> +++ b/include/linux/pkt_cls.h
> @@ -304,8 +304,8 @@ enum
>  	TCA_FW_UNSPEC,
>  	TCA_FW_CLASSID,
>  	TCA_FW_POLICE,
> -	TCA_FW_INDEV, /*  used by CONFIG_NET_CLS_IND */
> -	TCA_FW_ACT, /* used by CONFIG_NET_CLS_ACT */
> +	TCA_FW_INDEV,	/* used by NET_CLS_IND */
> +	TCA_FW_ACT,	/* used by NET_CLS_ACT */
>  	TCA_FW_MASK,

Again, surely we can improve the tool to not warn about these.  If nothing
else, the actual warning is not correct.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 22:39 ` [GIT PULL -tip] fix 41 'make headers_check' warnings Stephen Rothwell
@ 2009-01-19  1:20   ` Sam Ravnborg
  0 siblings, 0 replies; 22+ messages in thread
From: Sam Ravnborg @ 2009-01-19  1:20 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Jaswinder Singh Rajput, Ingo Molnar, Andrew Morton,
	x86 maintainers, LKML

On Mon, Jan 19, 2009 at 09:39:59AM +1100, Stephen Rothwell wrote:
> Hi Jaswinder,
> 
> On Sun, 18 Jan 2009 15:40:54 +0530 Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> >
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index 5571dbe..81fa255 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -1480,7 +1480,7 @@ struct v4l2_chip_ident_old {
> >  
> >  #if 1
> >  /* Experimental, meant for debugging, testing and internal use.
> > -   Only implemented if CONFIG_VIDEO_ADV_DEBUG is defined.
> > +   Only implemented if VIDEO_ADV_DEBUG is defined.
> >     You must be root to use these ioctls. Never use these in applications! */
> 
> Is there some reason we cannot fix the tool that does the checking for
> this case?

It could be done - but for now I have kept it very simple.
One day I should preprocess the headers or something to cover more
issues.

	Sam

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 22:50 ` Stephen Rothwell
@ 2009-01-19  2:30   ` Jaswinder Singh Rajput
  2009-01-19  3:53     ` Stephen Rothwell
  0 siblings, 1 reply; 22+ messages in thread
From: Jaswinder Singh Rajput @ 2009-01-19  2:30 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Ingo Molnar, Andrew Morton, Sam Ravnborg, x86 maintainers, LKML

On Mon, 2009-01-19 at 09:50 +1100, Stephen Rothwell wrote:
> Hi Jaswinder,
> 
> On Sun, 18 Jan 2009 15:40:54 +0530 Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> >
> > 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__ */
> >  
> >  #endif /* __LINUX_RAW_H */
> 
> Since MAX_RAW_MINORS has only one user (drivers/char/raw.c), why not
> just remove the define from here and add it it there?
> 

Already fixed in v3 as pointed by Ingo.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 23:53 ` Stephen Rothwell
@ 2009-01-19  2:31   ` Jaswinder Singh Rajput
  2009-01-19  3:48     ` Stephen Rothwell
  0 siblings, 1 reply; 22+ messages in thread
From: Jaswinder Singh Rajput @ 2009-01-19  2:31 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Ingo Molnar, Andrew Morton, Sam Ravnborg, x86 maintainers, LKML

On Mon, 2009-01-19 at 10:53 +1100, Stephen Rothwell wrote:
> Hi Jaswinder,
> 
> On Sun, 18 Jan 2009 15:40:54 +0530 Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> >
> > diff --git a/include/linux/pktcdvd.h b/include/linux/pktcdvd.h
> > index 04b4d73..277de8c 100644
> > --- 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__ */
> 
> This also only has one user (drivers/block/pktcdvd.c) so maybe it should
> be moved there.
> 

Already fixed in v3 as pointed by Ingo.

Thanks,

--
JSR


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-19  2:31   ` Jaswinder Singh Rajput
@ 2009-01-19  3:48     ` Stephen Rothwell
  2009-01-19  5:16       ` Jaswinder Singh Rajput
  0 siblings, 1 reply; 22+ messages in thread
From: Stephen Rothwell @ 2009-01-19  3:48 UTC (permalink / raw)
  To: Jaswinder Singh Rajput
  Cc: Ingo Molnar, Andrew Morton, Sam Ravnborg, x86 maintainers, LKML

[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]

Hi Jaswinder,

On Mon, 19 Jan 2009 08:01:14 +0530 Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
>
> On Mon, 2009-01-19 at 10:53 +1100, Stephen Rothwell wrote:
> > 
> > On Sun, 18 Jan 2009 15:40:54 +0530 Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> > >
> > > diff --git a/include/linux/pktcdvd.h b/include/linux/pktcdvd.h
> > > index 04b4d73..277de8c 100644
> > > --- 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__ */
> > 
> > This also only has one user (drivers/block/pktcdvd.c) so maybe it should
> > be moved there.
> > 
> 
> Already fixed in v3 as pointed by Ingo.

I was actually suggesting that you move the whole 

#if defined(CONFIG_CDROM_PKTCDVD_WCACHE)
#define USE_WCACHING		1
#else
#define USE_WCACHING		0
#endif

into drivers/block/pktcdvd.c as that is the only place USE_WCACHING is
used.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-19  2:30   ` Jaswinder Singh Rajput
@ 2009-01-19  3:53     ` Stephen Rothwell
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Rothwell @ 2009-01-19  3:53 UTC (permalink / raw)
  To: Jaswinder Singh Rajput
  Cc: Ingo Molnar, Andrew Morton, Sam Ravnborg, x86 maintainers, LKML

[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]

Hi Jaswinder,

On Mon, 19 Jan 2009 08:00:40 +0530 Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
>
> On Mon, 2009-01-19 at 09:50 +1100, Stephen Rothwell wrote:
> > Hi Jaswinder,
> > 
> > On Sun, 18 Jan 2009 15:40:54 +0530 Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> > >
> > > 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__ */
> > >  
> > >  #endif /* __LINUX_RAW_H */
> > 
> > Since MAX_RAW_MINORS has only one user (drivers/char/raw.c), why not
> > just remove the define from here and add it it there?
> > 
> 
> Already fixed in v3 as pointed by Ingo.

Again I was suggesting that the whole #define line could be moved to the
only C files it is used in.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-19  3:48     ` Stephen Rothwell
@ 2009-01-19  5:16       ` Jaswinder Singh Rajput
  0 siblings, 0 replies; 22+ messages in thread
From: Jaswinder Singh Rajput @ 2009-01-19  5:16 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Jaswinder Singh Rajput, Ingo Molnar, Andrew Morton, Sam Ravnborg,
	x86 maintainers, LKML

Hello Stephen,

On Mon, Jan 19, 2009 at 9:18 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>> > >
>> > > diff --git a/include/linux/pktcdvd.h b/include/linux/pktcdvd.h
>> > > index 04b4d73..277de8c 100644
>> > > --- 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__ */
>> >
>> > This also only has one user (drivers/block/pktcdvd.c) so maybe it should
>> > be moved there.
>> >
>>
>> Already fixed in v3 as pointed by Ingo.
>
> I was actually suggesting that you move the whole
>
> #if defined(CONFIG_CDROM_PKTCDVD_WCACHE)
> #define USE_WCACHING            1
> #else
> #define USE_WCACHING            0
> #endif
>
> into drivers/block/pktcdvd.c as that is the only place USE_WCACHING is
> used.
>

hmm, this is the out of scope of this patch and should be send in
different patch.

I think maintainers can handle this issue.

If they need my help, they can ping me ;-)

Thanks,

JSR

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 17:29       ` Ingo Molnar
@ 2009-01-21  0:58         ` Jaswinder Singh Rajput
  0 siblings, 0 replies; 22+ messages in thread
From: Jaswinder Singh Rajput @ 2009-01-21  0:58 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Sam Ravnborg, Andrew Morton, x86 maintainers, LKML

On Sun, 2009-01-18 at 18:29 +0100, Ingo Molnar wrote:
> * Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > On Sun, Jan 18, 2009 at 01:57:22PM +0100, Sam Ravnborg wrote:
> > > On Sun, Jan 18, 2009 at 12:02:21PM +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?
> > > 
> > > The original expression is misinterpreted by headers_check
> > > because we want the ac_ahz to stay if either of __KERNEL__
> > > or CONFIG_M68K is not defined.
> > > And unifdef does not optimize away the !defined(CONFIG_M68K)
> > > part - it has no knowledge that this is kernel internal.
> > > 
> > > So I am happy with Jaswinder's patch.
> > Almost happy.
> > I digged out my original patch and it looks like this:
> > -#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
> > +#ifndef __KERNEL__
> >         __u16           ac_ahz;                 /* AHZ */
> > +#else
> > +  #ifndef CONFIG_M68K
> > +       __u16           ac_ahz;                 /* AHZ */
> > +  #endif
> >  #endif
> > 
> > The indention can be discussed..
> > But the logic is simpler.
> 
> yes - that's the 3-block versus 2-block issue i mentioned to Jaswinder. 
> (the first version duplicated the same thing into 3 places - while the 
> logic only splits it in two)

my 7 lines patch was:
#ifdef __KERNEL__
#ifndef CONFIG_M68K
	__u16		ac_ahz;			/* AHZ */
#endif /* CONFIG_M68K */
#else /* __KERNEL__ */
	__u16		ac_ahz;			/* AHZ */
#endif /* __KERNEL__ */

and Sam's 7 line patch is:
#ifndef __KERNEL__
         __u16           ac_ahz;                 /* AHZ */
#else
  #ifndef CONFIG_M68K
         __u16           ac_ahz;                 /* AHZ */
  #endif
#endif

Ingo, please apply the patch which one you like, I just want to get rid of this warning ;-)

Thanks,
--
JSR


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [GIT PULL -tip] fix 41 'make headers_check' warnings
  2009-01-18 11:02 ` Ingo Molnar
  2009-01-18 11:26   ` Jaswinder Singh Rajput
  2009-01-18 12:57   ` Sam Ravnborg
@ 2009-01-21  1:27   ` Jaswinder Singh Rajput
  2009-01-21  5:53     ` Size of sector_t in userspace [Was: fix 41 'make headers_check' warnings] Sam Ravnborg
  2 siblings, 1 reply; 22+ messages in thread
From: Jaswinder Singh Rajput @ 2009-01-21  1:27 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Andrew Morton, Sam Ravnborg, x86 maintainers, LKML

Hello Sam,

On Sun, 2009-01-18 at 12:02 +0100, Ingo Molnar wrote:
> * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> > --- 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__ */
> 

What is your patch for this case.

Thanks,
--
JSR


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Size of sector_t in userspace [Was: fix 41 'make headers_check' warnings]
  2009-01-21  1:27   ` Jaswinder Singh Rajput
@ 2009-01-21  5:53     ` Sam Ravnborg
  2009-01-21  8:21       ` Jens Axboe
  0 siblings, 1 reply; 22+ messages in thread
From: Sam Ravnborg @ 2009-01-21  5:53 UTC (permalink / raw)
  To: Jaswinder Singh Rajput, axboe
  Cc: Ingo Molnar, Andrew Morton, x86 maintainers, LKML

Hi Jens.

In an attempt to fix some of the issues in our exported Headers jsr
encountered some strange stuff in types.h.

types.h is exported to userspace where we do not have access to CONFIG_*
symbols.
Despite this we use CONFGI_LBD to decide the size of sector_t like this:

#ifdef CONFIG_LBD
typedef u64 sector_t;
typedef u64 blkcnt_t;
#else
typedef unsigned long sector_t;
typedef unsigned long blkcnt_t;
#endif

But as CONFIG_LBD is never defined in userspace sector_t is now 32
bit on 32 bit boxes and 64 bit on 64 bit boxes (in userspace).

Is sector:t (and the companion blkcnt_t) really used by userspace?
If it is - what size is it expected to have?

	Sam

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Size of sector_t in userspace [Was: fix 41 'make headers_check' warnings]
  2009-01-21  5:53     ` Size of sector_t in userspace [Was: fix 41 'make headers_check' warnings] Sam Ravnborg
@ 2009-01-21  8:21       ` Jens Axboe
  2009-01-21 11:34         ` Sam Ravnborg
  0 siblings, 1 reply; 22+ messages in thread
From: Jens Axboe @ 2009-01-21  8:21 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Jaswinder Singh Rajput, Ingo Molnar, Andrew Morton,
	x86 maintainers, LKML

On Wed, Jan 21 2009, Sam Ravnborg wrote:
> Hi Jens.
> 
> In an attempt to fix some of the issues in our exported Headers jsr
> encountered some strange stuff in types.h.
> 
> types.h is exported to userspace where we do not have access to CONFIG_*
> symbols.
> Despite this we use CONFGI_LBD to decide the size of sector_t like this:
> 
> #ifdef CONFIG_LBD
> typedef u64 sector_t;
> typedef u64 blkcnt_t;
> #else
> typedef unsigned long sector_t;
> typedef unsigned long blkcnt_t;
> #endif
> 
> But as CONFIG_LBD is never defined in userspace sector_t is now 32
> bit on 32 bit boxes and 64 bit on 64 bit boxes (in userspace).
> 
> Is sector:t (and the companion blkcnt_t) really used by userspace?
> If it is - what size is it expected to have?

I don't think it's used by userspace, at least it cannot be used in
userspace <-> kernelspace interactions, since the size isn't fixed.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Size of sector_t in userspace [Was: fix 41 'make headers_check' warnings]
  2009-01-21  8:21       ` Jens Axboe
@ 2009-01-21 11:34         ` Sam Ravnborg
  0 siblings, 0 replies; 22+ messages in thread
From: Sam Ravnborg @ 2009-01-21 11:34 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Jaswinder Singh Rajput, Ingo Molnar, Andrew Morton,
	x86 maintainers, LKML

On Wed, Jan 21, 2009 at 09:21:25AM +0100, Jens Axboe wrote:
> On Wed, Jan 21 2009, Sam Ravnborg wrote:
> > Hi Jens.
> > 
> > In an attempt to fix some of the issues in our exported Headers jsr
> > encountered some strange stuff in types.h.
> > 
> > types.h is exported to userspace where we do not have access to CONFIG_*
> > symbols.
> > Despite this we use CONFGI_LBD to decide the size of sector_t like this:
> > 
> > #ifdef CONFIG_LBD
> > typedef u64 sector_t;
> > typedef u64 blkcnt_t;
> > #else
> > typedef unsigned long sector_t;
> > typedef unsigned long blkcnt_t;
> > #endif
> > 
> > But as CONFIG_LBD is never defined in userspace sector_t is now 32
> > bit on 32 bit boxes and 64 bit on 64 bit boxes (in userspace).
> > 
> > Is sector:t (and the companion blkcnt_t) really used by userspace?
> > If it is - what size is it expected to have?
> 
> I don't think it's used by userspace, at least it cannot be used in
> userspace <-> kernelspace interactions, since the size isn't fixed.

Thanks Jens.

In that case I suggest we should make it private to the kernel.
We should not expose types from the kernel that is not used
in the kerenl.

Jaswinder - move it inside a ifdef __KERNEL__ / endif block.
Reference Jens' answer above and Cc: Jens on the patch.

Thanks,
	Sam

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2009-01-21 11:33 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-18 10:10 [GIT PULL -tip] fix 41 'make headers_check' warnings Jaswinder Singh Rajput
2009-01-18 11:02 ` Ingo Molnar
2009-01-18 11:26   ` Jaswinder Singh Rajput
2009-01-18 12:26     ` Ingo Molnar
2009-01-18 12:57   ` Sam Ravnborg
2009-01-18 13:00     ` Sam Ravnborg
2009-01-18 17:29       ` Ingo Molnar
2009-01-21  0:58         ` Jaswinder Singh Rajput
2009-01-21  1:27   ` Jaswinder Singh Rajput
2009-01-21  5:53     ` Size of sector_t in userspace [Was: fix 41 'make headers_check' warnings] Sam Ravnborg
2009-01-21  8:21       ` Jens Axboe
2009-01-21 11:34         ` Sam Ravnborg
2009-01-18 22:39 ` [GIT PULL -tip] fix 41 'make headers_check' warnings Stephen Rothwell
2009-01-19  1:20   ` Sam Ravnborg
2009-01-18 22:50 ` Stephen Rothwell
2009-01-19  2:30   ` Jaswinder Singh Rajput
2009-01-19  3:53     ` Stephen Rothwell
2009-01-18 23:53 ` Stephen Rothwell
2009-01-19  2:31   ` Jaswinder Singh Rajput
2009-01-19  3:48     ` Stephen Rothwell
2009-01-19  5:16       ` Jaswinder Singh Rajput
2009-01-18 23:59 ` Stephen Rothwell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox