Patchwork [PATCH-2.6.28,final] byteorder: fix new headers for userspace

login
register
mail settings
Submitter Harvey Harrison
Date Dec. 18, 2008, 1:46 a.m.
Message ID <1229564779.6481.26.camel@brick>
Download mbox | patch
Permalink /patch/14603/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Harvey Harrison - Dec. 18, 2008, 1:46 a.m.
glibc headers define both __LITTLE_ENDIAN and __BIG_ENDIAN
which was tripping the check in linux/byteorder.h.  Let's
just stay out of userspace's way and use __KERN_{endian}

The old implementation got away with it as it kept the two
implementations in different headers and conditionally included
the right one.  The combined header does checks within each
function body and depends on only one being defined.

Converted the arches in mainline that have already moved to the
new header, as the other arches merge the will need simlar
fixups.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 arch/avr32/include/asm/byteorder.h |    2 +-
 arch/mips/include/asm/byteorder.h  |    4 +-
 arch/sh/include/asm/byteorder.h    |    4 +-
 arch/sparc/include/asm/byteorder.h |    2 +-
 include/linux/byteorder.h          |   84 ++++++++++++++++++------------------
 5 files changed, 48 insertions(+), 48 deletions(-)
Linus Torvalds - Dec. 18, 2008, 8:09 p.m.
On Wed, 17 Dec 2008, Harvey Harrison wrote:
>
> glibc headers define both __LITTLE_ENDIAN and __BIG_ENDIAN
> which was tripping the check in linux/byteorder.h.  Let's
> just stay out of userspace's way and use __KERN_{endian}

Gaah, ugly.

I think it would be better to just make it a config option, call it 
CONFIG_[LITTLE|BIG]_ENDIAN and set it up in the architecture Kconfig file. 
I think some of the people who can set it dynamically (or where it depends 
on the target machine) already effectively do that.

Hmm?

		Linus
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Torvalds - Dec. 18, 2008, 8:47 p.m.
On Thu, 18 Dec 2008, Linus Torvalds wrote:
> 
> I think it would be better to just make it a config option, call it 
> CONFIG_[LITTLE|BIG]_ENDIAN and set it up in the architecture Kconfig file. 
> I think some of the people who can set it dynamically (or where it depends 
> on the target machine) already effectively do that.

Never mind, since we end up having to export it to user space (do we?) we 
can't depend on the config.h file, nor can we pollute the name space.

Oh well.

		Linus
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Harvey Harrison - Dec. 18, 2008, 8:53 p.m.
On Thu, 2008-12-18 at 12:47 -0800, Linus Torvalds wrote:
> 
> On Thu, 18 Dec 2008, Linus Torvalds wrote:
> > 
> > I think it would be better to just make it a config option, call it 
> > CONFIG_[LITTLE|BIG]_ENDIAN and set it up in the architecture Kconfig file. 
> > I think some of the people who can set it dynamically (or where it depends 
> > on the target machine) already effectively do that.
> 
> Never mind, since we end up having to export it to user space (do we?) we 
> can't depend on the config.h file, nor can we pollute the name space.
> 
> Oh well.

I'm still having a bit of a think on this if we can avoid it.  But I
think we're stuck with needing a different symbol. :-/

Harvey

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arjan van de Ven - Dec. 19, 2008, 8:08 a.m.
On Thu, 18 Dec 2008 12:47:28 -0800 (PST)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> 
> On Thu, 18 Dec 2008, Linus Torvalds wrote:
> > 
> > I think it would be better to just make it a config option, call it 
> > CONFIG_[LITTLE|BIG]_ENDIAN and set it up in the architecture
> > Kconfig file. I think some of the people who can set it dynamically
> > (or where it depends on the target machine) already effectively do
> > that.
> 
> Never mind, since we end up having to export it to user space (do
> we?) we can't depend on the config.h file, nor can we pollute the
> name space.
> 

why are we exposing byte order functions to userspace in the first
place ????
Harvey Harrison - Dec. 19, 2008, 4:37 p.m.
On Fri, 2008-12-19 at 09:08 +0100, Arjan van de Ven wrote:
> On Thu, 18 Dec 2008 12:47:28 -0800 (PST)
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > 
> > 
> > On Thu, 18 Dec 2008, Linus Torvalds wrote:
> > > 
> > > I think it would be better to just make it a config option, call it 
> > > CONFIG_[LITTLE|BIG]_ENDIAN and set it up in the architecture
> > > Kconfig file. I think some of the people who can set it dynamically
> > > (or where it depends on the target machine) already effectively do
> > > that.
> > 
> > Never mind, since we end up having to export it to user space (do
> > we?) we can't depend on the config.h file, nor can we pollute the
> > name space.
> > 
> 
> why are we exposing byte order functions to userspace in the first
> place ????
> 

Hysterical raisons.  When I originally consolidated the byteorder stuff
I removed the userspace export.  HPA (CC'd) made the point that this
API (rightly or wrongly) was currently exported and used by userspace
and we shouldn't pull the rug out from under them.

I would have liked to un-export it, then we could also make linux/byteorder.h
the preferred include instead of asm/ ... but we appear to be stuck with
historical baggage here.

Crazy idea: could a byteorder-user.h be added that make headers-export
will pick up instead of byteorder.h that the kernel _actually_ uses?

Then we could keep exporting _something_ to avoid breaking userspace
and allow the version the kernel actually uses to be evolved a bit
more easily?

Harvey

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gerd Hoffmann - Jan. 6, 2009, 10:23 a.m.
Harvey Harrison wrote:
> glibc headers define both __LITTLE_ENDIAN and __BIG_ENDIAN
> which was tripping the check in linux/byteorder.h.  Let's
> just stay out of userspace's way and use __KERN_{endian}

Ping.  What is the status of this?

> Converted the arches in mainline that have already moved to the
> new header, as the other arches merge the will need simlar
> fixups.

In latest git powerpc, s390 and x86 are merged and need this fix too.

cheers,
  Gerd
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Harvey Harrison - Jan. 6, 2009, 4:41 p.m.
On Tue, 2009-01-06 at 11:23 +0100, Gerd Hoffmann wrote:
> Harvey Harrison wrote:
> > glibc headers define both __LITTLE_ENDIAN and __BIG_ENDIAN
> > which was tripping the check in linux/byteorder.h.  Let's
> > just stay out of userspace's way and use __KERN_{endian}
> 
> Ping.  What is the status of this?
> 
> > Converted the arches in mainline that have already moved to the
> > new header, as the other arches merge the will need simlar
> > fixups.
> 
> In latest git powerpc, s390 and x86 are merged and need this fix too.
> 
> cheers,
>   Gerd

I was waiting for the cris, xtensa and parisc arches to merge as they
have conversion patches in their arch trees.  I'll send the patchset out
today with the proviso it shouldn't be applied until those three have
merged.

Cheers,

Harvey

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/avr32/include/asm/byteorder.h b/arch/avr32/include/asm/byteorder.h
index 8e3af02..b7d6dd1 100644
--- a/arch/avr32/include/asm/byteorder.h
+++ b/arch/avr32/include/asm/byteorder.h
@@ -7,7 +7,7 @@ 
 #include <asm/types.h>
 #include <linux/compiler.h>
 
-#define __BIG_ENDIAN
+#define __KERN_BIG_ENDIAN
 #define __SWAB_64_THRU_32__
 
 #ifdef __CHECKER__
diff --git a/arch/mips/include/asm/byteorder.h b/arch/mips/include/asm/byteorder.h
index 2988d29..8ad8a5b 100644
--- a/arch/mips/include/asm/byteorder.h
+++ b/arch/mips/include/asm/byteorder.h
@@ -12,9 +12,9 @@ 
 #include <asm/types.h>
 
 #if defined(__MIPSEB__)
-# define __BIG_ENDIAN
+# define __KERN_BIG_ENDIAN
 #elif defined(__MIPSEL__)
-# define __LITTLE_ENDIAN
+# define __KERN_LITTLE_ENDIAN
 #else
 # error "MIPS, but neither __MIPSEB__, nor __MIPSEL__???"
 #endif
diff --git a/arch/sh/include/asm/byteorder.h b/arch/sh/include/asm/byteorder.h
index f5fa065..4aa5f1d 100644
--- a/arch/sh/include/asm/byteorder.h
+++ b/arch/sh/include/asm/byteorder.h
@@ -9,9 +9,9 @@ 
 #include <linux/types.h>
 
 #ifdef __LITTLE_ENDIAN__
-# define __LITTLE_ENDIAN
+# define __KERN_LITTLE_ENDIAN
 #else
-# define __BIG_ENDIAN
+# define __KERN_BIG_ENDIAN
 #endif
 
 #define __SWAB_64_THRU_32__
diff --git a/arch/sparc/include/asm/byteorder.h b/arch/sparc/include/asm/byteorder.h
index 5a70f13..5b8347e 100644
--- a/arch/sparc/include/asm/byteorder.h
+++ b/arch/sparc/include/asm/byteorder.h
@@ -4,7 +4,7 @@ 
 #include <asm/types.h>
 #include <asm/asi.h>
 
-#define __BIG_ENDIAN
+#define __KERN_BIG_ENDIAN
 
 #ifdef CONFIG_SPARC32
 #define __SWAB_64_THRU_32__
diff --git a/include/linux/byteorder.h b/include/linux/byteorder.h
index 29f002d..3599fbc 100644
--- a/include/linux/byteorder.h
+++ b/include/linux/byteorder.h
@@ -4,33 +4,33 @@ 
 #include <linux/types.h>
 #include <linux/swab.h>
 
-#if defined(__LITTLE_ENDIAN) && defined(__BIG_ENDIAN)
+#if defined(__KERN_LITTLE_ENDIAN) && defined(__KERN_BIG_ENDIAN)
 # error Fix asm/byteorder.h to define one endianness
 #endif
 
-#if !defined(__LITTLE_ENDIAN) && !defined(__BIG_ENDIAN)
+#if !defined(__KERN_LITTLE_ENDIAN) && !defined(__KERN_BIG_ENDIAN)
 # error Fix asm/byteorder.h to define arch endianness
 #endif
 
-#ifdef __LITTLE_ENDIAN
-# undef __LITTLE_ENDIAN
-# define __LITTLE_ENDIAN 1234
+#ifdef __KERN_LITTLE_ENDIAN
+# ifndef __LITTLE_ENDIAN
+#  define __LITTLE_ENDIAN 1234
+# endif
+# ifndef __LITTLE_ENDIAN_BITFIELD
+#  define __LITTLE_ENDIAN_BITFIELD
+# endif
 #endif
 
-#ifdef __BIG_ENDIAN
-# undef __BIG_ENDIAN
-# define __BIG_ENDIAN 4321
+#ifdef __KERN_BIG_ENDIAN
+# ifndef __BIG_ENDIAN
+#  define __BIG_ENDIAN 4321
+# endif
+# ifndef __BIG_ENDIAN_BITFIELD
+#  define __BIG_ENDIAN_BITFIELD
+# endif
 #endif
 
-#if defined(__LITTLE_ENDIAN) && !defined(__LITTLE_ENDIAN_BITFIELD)
-# define __LITTLE_ENDIAN_BITFIELD
-#endif
-
-#if defined(__BIG_ENDIAN) && !defined(__BIG_ENDIAN_BITFIELD)
-# define __BIG_ENDIAN_BITFIELD
-#endif
-
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 # define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
 # define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
 # define __le64_to_cpu(x) ((__force __u64)(__le64)(x))
@@ -46,7 +46,7 @@ 
 # define __cpu_to_be64(x) ((__force __be64)__swab64(x))
 #endif
 
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 # define __be16_to_cpu(x) ((__force __u16)(__be16)(x))
 # define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
 # define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
@@ -87,91 +87,91 @@ 
 
 static inline void __le16_to_cpus(__u16 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	__swab16s(p);
 #endif
 }
 
 static inline void __cpu_to_le16s(__u16 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	__swab16s(p);
 #endif
 }
 
 static inline void __le32_to_cpus(__u32 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	__swab32s(p);
 #endif
 }
 
 static inline void __cpu_to_le32s(__u32 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	__swab32s(p);
 #endif
 }
 
 static inline void __le64_to_cpus(__u64 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	__swab64s(p);
 #endif
 }
 
 static inline void __cpu_to_le64s(__u64 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	__swab64s(p);
 #endif
 }
 
 static inline void __be16_to_cpus(__u16 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	__swab16s(p);
 #endif
 }
 
 static inline void __cpu_to_be16s(__u16 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	__swab16s(p);
 #endif
 }
 
 static inline void __be32_to_cpus(__u32 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	__swab32s(p);
 #endif
 }
 
 static inline void __cpu_to_be32s(__u32 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	__swab32s(p);
 #endif
 }
 
 static inline void __be64_to_cpus(__u64 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	__swab64s(p);
 #endif
 }
 
 static inline void __cpu_to_be64s(__u64 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	__swab64s(p);
 #endif
 }
 
 static inline __u16 __le16_to_cpup(const __le16 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	return (__force __u16)*p;
 #else
 	return __swab16p((__force __u16 *)p);
@@ -180,7 +180,7 @@  static inline __u16 __le16_to_cpup(const __le16 *p)
 
 static inline __u32 __le32_to_cpup(const __le32 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	return (__force __u32)*p;
 #else
 	return __swab32p((__force __u32 *)p);
@@ -189,7 +189,7 @@  static inline __u32 __le32_to_cpup(const __le32 *p)
 
 static inline __u64 __le64_to_cpup(const __le64 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	return (__force __u64)*p;
 #else
 	return __swab64p((__force __u64 *)p);
@@ -198,7 +198,7 @@  static inline __u64 __le64_to_cpup(const __le64 *p)
 
 static inline __le16 __cpu_to_le16p(const __u16 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	return (__force __le16)*p;
 #else
 	return (__force __le16)__swab16p(p);
@@ -207,7 +207,7 @@  static inline __le16 __cpu_to_le16p(const __u16 *p)
 
 static inline __le32 __cpu_to_le32p(const __u32 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	return (__force __le32)*p;
 #else
 	return (__force __le32)__swab32p(p);
@@ -216,7 +216,7 @@  static inline __le32 __cpu_to_le32p(const __u32 *p)
 
 static inline __le64 __cpu_to_le64p(const __u64 *p)
 {
-#ifdef __LITTLE_ENDIAN
+#ifdef __KERN_LITTLE_ENDIAN
 	return (__force __le64)*p;
 #else
 	return (__force __le64)__swab64p(p);
@@ -225,7 +225,7 @@  static inline __le64 __cpu_to_le64p(const __u64 *p)
 
 static inline __u16 __be16_to_cpup(const __be16 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	return (__force __u16)*p;
 #else
 	return __swab16p((__force __u16 *)p);
@@ -234,7 +234,7 @@  static inline __u16 __be16_to_cpup(const __be16 *p)
 
 static inline __u32 __be32_to_cpup(const __be32 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	return (__force __u32)*p;
 #else
 	return __swab32p((__force __u32 *)p);
@@ -243,7 +243,7 @@  static inline __u32 __be32_to_cpup(const __be32 *p)
 
 static inline __u64 __be64_to_cpup(const __be64 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	return (__force __u64)*p;
 #else
 	return __swab64p((__force __u64 *)p);
@@ -252,7 +252,7 @@  static inline __u64 __be64_to_cpup(const __be64 *p)
 
 static inline __be16 __cpu_to_be16p(const __u16 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	return (__force __be16)*p;
 #else
 	return (__force __be16)__swab16p(p);
@@ -261,7 +261,7 @@  static inline __be16 __cpu_to_be16p(const __u16 *p)
 
 static inline __be32 __cpu_to_be32p(const __u32 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	return (__force __be32)*p;
 #else
 	return (__force __be32)__swab32p(p);
@@ -270,7 +270,7 @@  static inline __be32 __cpu_to_be32p(const __u32 *p)
 
 static inline __be64 __cpu_to_be64p(const __u64 *p)
 {
-#ifdef __BIG_ENDIAN
+#ifdef __KERN_BIG_ENDIAN
 	return (__force __be64)*p;
 #else
 	return (__force __be64)__swab64p(p);