[patch 0/7] Use strict kernel types to fix the world

Previous thread: [patch 2/7] make most exported headers use strict integer types by arnd on Wednesday, February 25, 2009 - 4:51 pm. (1 message)

Next thread: by antslpysm on Wednesday, February 25, 2009 - 4:42 pm. (1 message)
From: arnd
Date: Wednesday, February 25, 2009 - 4:51 pm

I've gone through the files again and fixed up all
non-strict types I could find. I did not check all
architectures for this though.

I split out netfilter, DRM and MTD, because of both
size and potentially controversial changes.

	Arnd <><

--

From: arnd
Date: Wednesday, February 25, 2009 - 4:51 pm

The MTD headers traditionally use stdint types rather than
the kernel integer types. This converts them to do the
same as all the others.

Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/jffs2.h    |   26 ++++++++--------
 include/mtd/inftl-user.h |   36 ++++++++++++------------
 include/mtd/jffs2-user.h |    4 +-
 include/mtd/mtd-abi.h    |   64 +++++++++++++++++++++---------------------
 include/mtd/nftl-user.h  |   30 ++++++++++----------
 include/mtd/ubi-user.h   |   70 +++++++++++++++++++++++-----------------------
 6 files changed, 115 insertions(+), 115 deletions(-)

Index: linux-2.6/include/linux/jffs2.h
===================================================================
--- linux-2.6.orig/include/linux/jffs2.h
+++ linux-2.6/include/linux/jffs2.h
@@ -12,6 +12,7 @@
 #ifndef __LINUX_JFFS2_H__
 #define __LINUX_JFFS2_H__
 
+#include <linux/types.h>
 #include <linux/magic.h>
 
 /* You must include something which defines the C99 uintXX_t types. 
@@ -91,15 +92,15 @@
    byteswapping */
 
 typedef struct {
-	uint32_t v32;
+	__u32 v32;
 } __attribute__((packed)) jint32_t;
 
 typedef struct {
-	uint32_t m;
+	__u32 m;
 } __attribute__((packed)) jmode_t;
 
 typedef struct {
-	uint16_t v16;
+	__u16 v16;
 } __attribute__((packed)) jint16_t;
 
 struct jffs2_unknown_node
@@ -121,12 +122,12 @@ struct jffs2_raw_dirent
 	jint32_t version;
 	jint32_t ino; /* == zero for unlink */
 	jint32_t mctime;
-	uint8_t nsize;
-	uint8_t type;
-	uint8_t unused[2];
+	__u8 nsize;
+	__u8 type;
+	__u8 unused[2];
 	jint32_t node_crc;
 	jint32_t name_crc;
-	uint8_t name[0];
+	__u8 name[0];
 };
 
 /* The JFFS2 raw inode structure: Used for storage on physical media.  */
@@ -153,12 +154,12 @@ struct jffs2_raw_inode
 	jint32_t offset;     /* Where to begin to write.  */
 	jint32_t csize;      /* (Compressed) data size */
 	jint32_t dsize;	     /* Size of the node's data. (after decompression) */
-	uint8_t compr;       ...
From: Thiago Galesi
Date: Wednesday, February 25, 2009 - 5:32 pm

I don't know if the idea is to do all at once, but MTD has a lot of
things like u_char as well

-- 
-
Thiago Galesi


--

From: Arnd Bergmann
Date: Wednesday, February 25, 2009 - 5:34 pm

I could not find any of those in the exported parts of the MTD headers,
only inside of #ifdef __KERNEL__, where it doesn't do any harm.

	Arnd <><
--

From: arnd
Date: Wednesday, February 25, 2009 - 4:51 pm

With the last used of non-strict names gone from the
exported header files, we can remove the old libc5
compatibility cruft from our headers and only export
strict types.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/asm-generic/statfs.h |    5 +++--
 include/linux/types.h        |   13 ++-----------
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/include/asm-generic/statfs.h b/include/asm-generic/statfs.h
index 6129d68..3b4fb3e 100644
--- a/include/asm-generic/statfs.h
+++ b/include/asm-generic/statfs.h
@@ -1,8 +1,9 @@
 #ifndef _GENERIC_STATFS_H
 #define _GENERIC_STATFS_H
 
-#ifndef __KERNEL_STRICT_NAMES
-# include <linux/types.h>
+#include <linux/types.h>
+
+#ifdef __KERNEL__
 typedef __kernel_fsid_t	fsid_t;
 #endif
 
diff --git a/include/linux/types.h b/include/linux/types.h
index 712ca53..6508ba1 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -11,7 +11,7 @@
 #include <linux/posix_types.h>
 #include <asm/types.h>
 
-#ifndef __KERNEL_STRICT_NAMES
+#ifdef __KERNEL__
 
 typedef __u32 __kernel_dev_t;
 
@@ -29,7 +29,6 @@ typedef __kernel_timer_t	timer_t;
 typedef __kernel_clockid_t	clockid_t;
 typedef __kernel_mqd_t		mqd_t;
 
-#ifdef __KERNEL__
 typedef _Bool			bool;
 
 typedef __kernel_uid32_t	uid_t;
@@ -45,14 +44,6 @@ typedef __kernel_old_uid_t	old_uid_t;
 typedef __kernel_old_gid_t	old_gid_t;
 #endif /* CONFIG_UID16 */
 
-/* libc5 includes this file to define uid_t, thus uid_t can never change
- * when it is included by non-kernel code
- */
-#else
-typedef __kernel_uid_t		uid_t;
-typedef __kernel_gid_t		gid_t;
-#endif /* __KERNEL__ */
-
 #if defined(__GNUC__)
 typedef __kernel_loff_t		loff_t;
 #endif
@@ -154,7 +145,7 @@ typedef unsigned long blkcnt_t;
 #define pgoff_t unsigned long
 #endif
 
-#endif /* __KERNEL_STRICT_NAMES */
+#endif /* __KERNEL__ */
 
 /*
  * Below are truly Linux-specific types that should never collide with
-- 
1.5.6.3

-- 

--

From: arnd
Date: Wednesday, February 25, 2009 - 4:51 pm

Netfilter traditionally uses BSD integer types in its
interface headers. This changes it to use the Linux
strict integer types, like everyone else.

Cc: netfilter-devel@vger.kernel.org
Cc: coreteam@netfilter.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/netfilter/nf_conntrack_tcp.h |    6 +++-
 include/linux/netfilter/nfnetlink.h        |    4 +-
 include/linux/netfilter/nfnetlink_compat.h |    7 ++++-
 include/linux/netfilter/nfnetlink_log.h    |   32 ++++++++++++------------
 include/linux/netfilter/nfnetlink_queue.h  |   24 +++++++++---------
 include/linux/netfilter/x_tables.h         |   30 ++++++++++++-----------
 include/linux/netfilter/xt_CLASSIFY.h      |    4 ++-
 include/linux/netfilter/xt_CONNMARK.h      |    8 ++++--
 include/linux/netfilter/xt_CONNSECMARK.h   |    4 ++-
 include/linux/netfilter/xt_DSCP.h          |    7 +++--
 include/linux/netfilter/xt_MARK.h          |    6 +++-
 include/linux/netfilter/xt_NFLOG.h         |   12 +++++----
 include/linux/netfilter/xt_NFQUEUE.h       |    4 ++-
 include/linux/netfilter/xt_RATEEST.h       |    6 +++-
 include/linux/netfilter/xt_SECMARK.h       |    6 +++-
 include/linux/netfilter/xt_TCPMSS.h        |    4 ++-
 include/linux/netfilter/xt_connbytes.h     |    6 +++-
 include/linux/netfilter/xt_connmark.h      |    8 ++++--
 include/linux/netfilter/xt_conntrack.h     |   12 ++++----
 include/linux/netfilter/xt_dccp.h          |   14 ++++++----
 include/linux/netfilter/xt_dscp.h          |   12 +++++----
 include/linux/netfilter/xt_esp.h           |    6 +++-
 include/linux/netfilter/xt_hashlimit.h     |   32 +++++++++++++-----------
 include/linux/netfilter/xt_iprange.h       |    4 ++-
 include/linux/netfilter/xt_length.h        |    6 +++-
 include/linux/netfilter/xt_limit.h         |   10 ++++---
 include/linux/netfilter/xt_mark.h          |    8 ++++--
 include/linux/netfilter/xt_multiport.h     |   18 +++++++------
 include/linux/netfilter/xt_owner.h         |    8 ++++--
 ...
From: Jan Engelhardt
Date: Wednesday, February 25, 2009 - 5:10 pm

I _strongly disagree_ with this move. Userspace also has the uintX
types via <stdint.h>/<cstdint>, and now you are adding a dependency
on linux/types.h, not to mention that your step can lead to compile
time piling up.

IMHO, __uXX should be replaced by uintX_t, but a move this great I
will leave to future generations because there is just too much
persisting opinions wrt. such proposal. As such I'd like to join
--

From: H. Peter Anvin
Date: Wednesday, February 25, 2009 - 5:14 pm

We *CAN'T* replace __uXX with uintX_t.  Period, full stop, end of story. 
  It isn't possible to do universally, and doing it non-universally is 
just silly.  Nor is it likely that this adds anything to the compile 
time, since most realistic programs will have a dependency on 
<linux/types.h> through some indirect path.

Now, I have to say I'd personally would prefer if make headers_install 
did this transformation mechanically for the common integer types, 
instead of requiring it to have the source code formatted in a specific 
fashion.

	-hpa
--

From: Jan Engelhardt
Date: Wednesday, February 25, 2009 - 5:39 pm

Would the mindful master please elaborate why this is so, if it is
indeed not a personal decision.
--

From: H. Peter Anvin
Date: Wednesday, February 25, 2009 - 6:02 pm

POSIX has strict rules as to what symbols you are allowed to present 
into the user-visible namespace.  This means you can't use these types 
for any header that may be indirectly included.

Since we can't use them in general, it's silly to use them for a 
minority of headers, which would then have different exposure rules than 
people otherwise can expect.

	-hpa

--

From: H. Peter Anvin
Date: Wednesday, February 25, 2009 - 5:24 pm

Not to mention the fact that the standard type is uint8_t, not u_int8_t 
as the file currently have, which makes it double broken.
	
	-hpa
--

From: David Miller
Date: Wednesday, February 25, 2009 - 5:55 pm

From: Jan Engelhardt <jengelh@medozas.de>

Disagreed, I think we should have done what Arnd is doing
a long time ago.

Ending up with linux/types.h in userspace for these kinds of
interfaces is already a fore-gone conclusion, it happens
already whether you like it or not.

And existing apps will work just fine, since they are already
getting stdint.h

As for compile time, since you're already getting linux/types.h
in your apps it's a non-argument.  But even if it was, what
are you compiling netfilter utilities on?  A VAX?
--

From: David Miller
Date: Wednesday, February 25, 2009 - 5:59 pm

From: "H. Peter Anvin" <hpa@zytor.com>

Yes, ignore Jan, I think he forgot to take his medication today:

Acked-by: David S. Miller <davem@davemloft.net>
--

From: H. Peter Anvin
Date: Wednesday, February 25, 2009 - 5:58 pm

I take it that you're OK that we merge Arnd's patchset?

	-hpa
--

From: arnd
Date: Wednesday, February 25, 2009 - 4:51 pm

The drm headers are traditionally shared with BSD and
could not use the strict linux integer types. This is
over now, so we can use our own types now.

Cc: David Airlie <airlied@linux.ie>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/drm/drm.h        |   21 +++---
 include/drm/drm_mode.h   |  153 ++++++++++++++++++++++-----------------------
 include/drm/i915_drm.h   |  140 +++++++++++++++++++++---------------------
 include/drm/mga_drm.h    |   18 +++---
 include/drm/radeon_drm.h |    4 +-
 include/drm/via_drm.h    |   42 +++++++------
 6 files changed, 190 insertions(+), 188 deletions(-)

diff --git a/include/drm/drm.h b/include/drm/drm.h
index 8e77357..7cb50bd 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -36,8 +36,7 @@
 #ifndef _DRM_H_
 #define _DRM_H_
 
-#if defined(__KERNEL__)
-#endif
+#include <linux/types.h>
 #include <asm/ioctl.h>		/* For _IO* macros */
 #define DRM_IOCTL_NR(n)		_IOC_NR(n)
 #define DRM_IOC_VOID		_IOC_NONE
@@ -497,8 +496,8 @@ union drm_wait_vblank {
  * \sa drmModesetCtl().
  */
 struct drm_modeset_ctl {
-	uint32_t crtc;
-	uint32_t cmd;
+	__u32 crtc;
+	__u32 cmd;
 };
 
 /**
@@ -574,29 +573,29 @@ struct drm_set_version {
 /** DRM_IOCTL_GEM_CLOSE ioctl argument type */
 struct drm_gem_close {
 	/** Handle of the object to be closed. */
-	uint32_t handle;
-	uint32_t pad;
+	__u32 handle;
+	__u32 pad;
 };
 
 /** DRM_IOCTL_GEM_FLINK ioctl argument type */
 struct drm_gem_flink {
 	/** Handle for the object being named */
-	uint32_t handle;
+	__u32 handle;
 
 	/** Returned global name */
-	uint32_t name;
+	__u32 name;
 };
 
 /** DRM_IOCTL_GEM_OPEN ioctl argument type */
 struct drm_gem_open {
 	/** Name of object being opened */
-	uint32_t name;
+	__u32 name;
 
 	/** Returned handle for the object */
-	uint32_t handle;
+	__u32 handle;
 
 	/** Returned size of the object */
-	uint64_t size;
+	__u64 size;
 };
 
 #include "drm_mode.h"
diff --git a/include/drm/drm_mode.h ...
From: arnd
Date: Wednesday, February 25, 2009 - 4:51 pm

Most of this header file makes no sense in user space
and should not be exported.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/coda_psdev.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/linux/coda_psdev.h b/include/linux/coda_psdev.h
index 07ae8f8..babce2e 100644
--- a/include/linux/coda_psdev.h
+++ b/include/linux/coda_psdev.h
@@ -6,6 +6,8 @@
 #define CODA_PSDEV_MAJOR 67
 #define MAX_CODADEVS  5	   /* how many do we allow */
 
+#ifdef __KERNEL__
+
 struct kstatfs;
 
 /* communication pending/processing queues */
@@ -90,3 +92,4 @@ struct upc_req {
 extern struct venus_comm coda_comms[];
 
 #endif
+#endif
-- 
1.5.6.3

-- 

--

From: arnd
Date: Wednesday, February 25, 2009 - 4:51 pm

A number of standard posix types are used in exported headers, which
is not allowed if __STRICT_KERNEL_NAMES is defined. In order to
get rid of the non-__STRICT_KERNEL_NAMES part and to make sane headers
the default, we have to change them all to safe types.

There are also still some leftovers in reiserfs_fs.h, elfcore.h
and coda.h, but these files have not compiled in user space for
a long time.

This leaves out the various integer types ({u_,u,}int{8,16,32,64}_t),
which we take care of separately.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/asm-generic/fcntl.h               |   12 ++++++------
 include/asm-generic/siginfo.h             |   14 +++++++-------
 include/linux/agpgart.h                   |   14 +++++++-------
 include/linux/cn_proc.h                   |   20 ++++++++++----------
 include/linux/cyclades.h                  |    6 +++---
 include/linux/dvb/video.h                 |    2 +-
 include/linux/if_pppol2tp.h               |    2 +-
 include/linux/mroute6.h                   |    2 +-
 include/linux/netfilter_ipv4/ipt_owner.h  |    8 ++++----
 include/linux/netfilter_ipv6/ip6t_owner.h |    8 ++++----
 include/linux/ppp_defs.h                  |    4 ++--
 include/linux/suspend_ioctls.h            |   10 +++++-----
 include/linux/time.h                      |    8 ++++----
 include/linux/times.h                     |    8 ++++----
 include/linux/utime.h                     |    4 ++--
 include/linux/xfrm.h                      |    2 +-
 include/mtd/mtd-abi.h                     |    4 ++--
 include/sound/asound.h                    |    4 ++--
 18 files changed, 66 insertions(+), 66 deletions(-)

Index: linux-2.6/include/asm-generic/fcntl.h
===================================================================
--- linux-2.6.orig/include/asm-generic/fcntl.h
+++ linux-2.6/include/asm-generic/fcntl.h
@@ -117,9 +117,9 @@
 struct flock {
 	short	l_type;
 ...
From: H. Peter Anvin
Date: Wednesday, February 25, 2009 - 5:02 pm

I take it this supercedes your previous monolithic patch?

	-hpa
--

From: Arnd Bergmann
Date: Wednesday, February 25, 2009 - 5:24 pm

Patch 1/7 supercedes my previous patch and is still as monolithic,
except for the separate 6/7 hunk. I only split the integer type
patches by subsystem, but I did these from scratch now.

Patch 7/7 is a new one, based on your input from the last time
we discussed it.

	Arnd <><
--

From: H. Peter Anvin
Date: Wednesday, February 25, 2009 - 5:30 pm

6/7 we already had in -tip, apparently.

I'm really of two minds regarding the patches that replace pure data 
types (2/7-5/7).  Part of me thinks it would be better to do this via a 
script in make headers_install, but another part of me thinks that that 
is a recipe for missing includes.

However, if subsystem maintainers are sharing headers with other 
platforms, it's probably the only sane road to go.

	-hpa

--

From: Arnd Bergmann
Date: Wednesday, February 25, 2009 - 5:52 pm

The only file I found that is obviously shared across operating systems
is linux/coda.h, and I completely left that one alone on the basis
that the hacks in there should still work with the new linux/types.h.

Doing an automated conversion on coda.h would guarantee trouble, which
I see as an argument for doing the manual approach in general.

The changes outside of mtd, netfilter, drm and pfkeyv2.h are actually
pretty minimal. For reference, all other patches touching those files
since 2.6.28 have a combined diffstat of 

 include/drm/drm.h                      |   26 +++-
 include/drm/drm_mode.h                 |  271 ++++++++++++++++++++++++++++++++
 include/drm/i915_drm.h                 |   43 +++++-
 include/linux/agpgart.h                |    1 -
 include/linux/cyclades.h               |    2 -
 include/linux/dvb/audio.h              |    5 -
 include/linux/dvb/video.h              |    7 +-
 include/linux/if_pppol2tp.h            |    2 +-
 include/linux/matroxfb.h               |    2 +-
 include/linux/mroute6.h                |   26 +++-
 include/linux/netfilter/x_tables.h     |    2 +-
 include/linux/netfilter/xt_conntrack.h |    1 +
 include/linux/pkt_sched.h              |   18 ++
 include/linux/ppp_defs.h               |    2 +
 include/linux/time.h                   |    1 +
 include/linux/types.h                  |   24 ++--
 include/linux/xfrm.h                   |   14 ++
 include/mtd/inftl-user.h               |    2 +
 include/mtd/ubi-user.h                 |  134 +++++++++++++---
 include/sound/asound.h                 |    1 +
 20 files changed, 530 insertions(+), 54 deletions(-)

As long as the netfilter, mtd and drm maintainers agree, I don't see
anything holding up the convert-everything-now approach.

	Arnd <><
--

From: Ingo Molnar
Date: Wednesday, February 25, 2009 - 5:56 pm

In addition to that we could also keep tip:core/header-fixes 
feature-free and pullable. So should any maintainer run into 
conflicts in that area the branch could be pulled into that 
tree. (the branch is focused on fixing the situation so 
generally pullable.)

	Ingo
--

Previous thread: [patch 2/7] make most exported headers use strict integer types by arnd on Wednesday, February 25, 2009 - 4:51 pm. (1 message)

Next thread: by antslpysm on Wednesday, February 25, 2009 - 4:42 pm. (1 message)