linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
       [not found] <20030506110259.A29633@infradead.org>
@ 2003-05-06 10:24 ` Thomas Horsten
  0 siblings, 0 replies; 44+ messages in thread
From: Thomas Horsten @ 2003-05-06 10:24 UTC (permalink / raw)
  To: marcelo; +Cc: Christoph Hellwig, linux-kernel

Hi Marcelo,

Here is a patch to fix the following problem (revised as Christoph
suggested): In 2.4.21-rc1 some inline functions are added to
asm-i386/byteorder.h. When __STRICT_ANSI__ is defined, __u64 doesn't get
defined by asm-i386/types.h, but it is used in one of the new inline
functions, __arch__swab64() - this causes a compile error for any program
that includes linux/cdrom.h and is built with -ansi. See also Christoph's
other comments on the list.

On Tue, 6 May 2003, Christoph Hellwig wrote:
> [..]
> You might want to reorder the code a bit to have only one
> __STRICT_ANSI__ ifdef, but else it looks fine.

// Thomas


--- linux-2.4.21-rc1-orig/include/asm-i386/byteorder.h	2003-05-06 09:52:33.000000000 +0100
+++ linux-2.4.21-rc1-ac4/include/asm-i386/byteorder.h	2003-05-06 11:20:01.000000000 +0100
@@ -34,7 +34,7 @@
 		return x;
 }

-
+#ifndef __STRICT_ANSI__
 static inline __u64 ___arch__swab64(__u64 val)
 {
 	union {
@@ -54,12 +54,14 @@
 	return v.u;
 }

+#define __BYTEORDER_HAS_U64__
 #define __arch__swab64(x) ___arch__swab64(x)
+
+#endif /* !__STRICT_ANSI__ */
+
 #define __arch__swab32(x) ___arch__swab32(x)
 #define __arch__swab16(x) ___arch__swab16(x)

-#define __BYTEORDER_HAS_U64__
-
 #endif /* __GNUC__ */

 #include <linux/byteorder/little_endian.h>


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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 22:31                 ` David S. Miller
@ 2003-11-06 23:40                   ` H. Peter Anvin
  0 siblings, 0 replies; 44+ messages in thread
From: H. Peter Anvin @ 2003-11-06 23:40 UTC (permalink / raw)
  To: linux-kernel

Followup to:  <20031106143110.6231ecde.davem@redhat.com>
By author:    "David S. Miller" <davem@redhat.com>
In newsgroup: linux.dev.kernel
>
> On 6 Nov 2003 12:40:55 -0800
> "H. Peter Anvin" <hpa@zytor.com> wrote:
> 
> > Note that "long long" (the underlying type) is valid
> > standards-compliant C99.  gcc can handle it when in C89 mode if
> > defined as __extension__ long long IIRC.
> 
> That's correct and I've suggested this.
> 
> But keep in mind that people with non-GCC compilers will then
> start complaining.
>

So...

#ifdef __GNUC__
# define __gcc_extension __extension__
#else
# define __gcc_extension
#endif

typedef __gcc_extension signed long long s32;
typedef __gcc_extension unsigned long long u32;

	-hpa


-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
If you send me mail in HTML format I will assume it's spam.
"Unix gives you enough rope to shoot yourself in the foot."
Architectures needed: ia64 m68k mips64 ppc ppc64 s390 s390x sh v850 x86-64

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 20:40               ` H. Peter Anvin
@ 2003-11-06 22:31                 ` David S. Miller
  2003-11-06 23:40                   ` H. Peter Anvin
  0 siblings, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-11-06 22:31 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: linux-kernel

On 6 Nov 2003 12:40:55 -0800
"H. Peter Anvin" <hpa@zytor.com> wrote:

> Note that "long long" (the underlying type) is valid
> standards-compliant C99.  gcc can handle it when in C89 mode if
> defined as __extension__ long long IIRC.

That's correct and I've suggested this.

But keep in mind that people with non-GCC compilers will then
start complaining.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 21:18                   ` David S. Miller
@ 2003-11-06 21:59                     ` Martin Schlemmer
  0 siblings, 0 replies; 44+ messages in thread
From: Martin Schlemmer @ 2003-11-06 21:59 UTC (permalink / raw)
  To: David S. Miller; +Cc: KML

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

On Thu, 2003-11-06 at 23:18, David S. Miller wrote:
> On Thu, 06 Nov 2003 23:18:55 +0200
> Martin Schlemmer <azarah@gentoo.org> wrote:
> 
> > Ok - say for instance then you were to write the abi headers - how would
> > you handle a case like this that -ansi forbid type long long, but it
> > have to be in the struct userspace uses to pass data to the
> > kernel/device ?
> 
> I can tell you what cannot happen.  You absolutely can't consider
> ideas like using '__u32 val[2];' and accessor macros, long long or
> whatever type the platform uses for __u64 has different alignment
> constraints than the '__u32 val[2]' array thing would.
> 
> I believe there is a way to work around this by using the
> __extension__ keyword when defining the __u64 typedefs.  Someone
> should try playing with that.  But this is only going to work when
> the compiler is GCC.

Yes, I do understand, and no, I did not try to get on that
nerve =)

The thing is just that you guys cannot decide if left, or
right is the best path to take, and that do create some
confusion, especially when you want to do the proper fix,
and a few things are falling apart around you =)

And patching it the wrong way, and then hitting one of your
possible quirks, will make things even worse, as if nobody
can remember about this, then it might be a very hard job
to track it, as you will be the only ones with this issue.

Some upstream userland authors have already done come up
with the following 'fix', which you may or may not approve
of:

--
#undef __STRICT_ANSI__
#include <linux/cdrom.h>
#define __STRICT_ANSI__
--

I guess the easier option might just be to drop -ansi :/


Thanks,

-- 

Martin Schlemmer




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 21:18                 ` Martin Schlemmer
  2003-11-06 21:18                   ` David S. Miller
@ 2003-11-06 21:24                   ` Daniel Jacobowitz
  1 sibling, 0 replies; 44+ messages in thread
From: Daniel Jacobowitz @ 2003-11-06 21:24 UTC (permalink / raw)
  To: KML

On Thu, Nov 06, 2003 at 11:18:55PM +0200, Martin Schlemmer wrote:
> On Thu, 2003-11-06 at 22:27, David S. Miller wrote:
> > On Thu, 06 Nov 2003 22:29:12 +0200
> > Martin Schlemmer <azarah@gentoo.org> wrote:
> > 
> > > If you look at asm/types.h, u64 is kernel only namespace, so in
> > > theory that code will not be in userspace.
> > 
> > Replace u64 with __u64 in my examples, the point still stances.
> > 
> > 
> > > #else
> > > <code without __u64>
> > > ..
> > > #endif
> > 
> > This may not be possible.  You cannot account for every internal
> > thing that kernel header routines might need to do or work with.
> > Many structures, which the userspace can't control the layout
> > of etc., makes use of the __u64 type, and we can't just turn off
> > all those things just because -ansi was specified.
> > 
> > We're talking about things like structures that define the userspace
> > ABI into the kernel, they use things like __u64.  So what effectively
> > this means is that when you compile with -ansi you are effectively
> > turning off access to several userspace ABIs into the kernel.
> > 
> > And this isn't going to be only some obscrure feature like some
> > CDROM ioctl or whatever, things like "struct stat" use the 64-bit types
> > either directly or indirectly.
> 
> Ok - say for instance then you were to write the abi headers - how would
> you handle a case like this that -ansi forbid type long long, but it
> have to be in the struct userspace uses to pass data to the
> kernel/device ?

As someone else mentioned, you can use __extension__ if GNUC is
defined.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 20:05           ` David S. Miller
  2003-11-06 20:29             ` Martin Schlemmer
@ 2003-11-06 21:21             ` Daniel Jacobowitz
  1 sibling, 0 replies; 44+ messages in thread
From: Daniel Jacobowitz @ 2003-11-06 21:21 UTC (permalink / raw)
  To: linux-kernel

On Thu, Nov 06, 2003 at 12:05:48PM -0800, David S. Miller wrote:
> On Thu, 06 Nov 2003 22:09:29 +0200
> Martin Schlemmer <azarah@gentoo.org> wrote:
> 
> > On Thu, 2003-11-06 at 21:37, David S. Miller wrote:
> > > Let's say that you end up using some inline function
> > > that takes u32 arguments, and internally it uses
> > > u64 types to speed up the calculation or make it more
> > > accurate or something like that.
> > 
> > So basically only in cases where the stuff in byteorder.h
> > was not inlined ... ?
> 
> No, exactly in the cases where it _IS_ inlined.  Imagine
> this:
> 
> static inline u32 swab_foo(u32 a, u32 b)
> {
> 	u64 tmp = ((u64)a<<32) | ((u64)b);
> 	u32 retval;
> 
> 	retval = compute(tmp);
> 
> 	return retval;
> }
> 
> If that's in a kernel header somewhere, and you build with -ansi,
> you lose.

In general the inlines should be __KERNEL__'d anyway.  In any case, the
prior art in <linux/types.h> is:

#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
typedef         __u64           uint64_t;
typedef         __u64           u_int64_t;
typedef         __s64           int64_t;
#endif

Or did I miss something at the beginning of this conversation?


[Debian is already using similar patches, which disable the 64-bit
swabbing in __STRICT_ANSI__.]

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 20:27               ` David S. Miller
@ 2003-11-06 21:18                 ` Martin Schlemmer
  2003-11-06 21:18                   ` David S. Miller
  2003-11-06 21:24                   ` Daniel Jacobowitz
  0 siblings, 2 replies; 44+ messages in thread
From: Martin Schlemmer @ 2003-11-06 21:18 UTC (permalink / raw)
  To: David S. Miller; +Cc: KML

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

On Thu, 2003-11-06 at 22:27, David S. Miller wrote:
> On Thu, 06 Nov 2003 22:29:12 +0200
> Martin Schlemmer <azarah@gentoo.org> wrote:
> 
> > If you look at asm/types.h, u64 is kernel only namespace, so in
> > theory that code will not be in userspace.
> 
> Replace u64 with __u64 in my examples, the point still stances.
> 
> 
> > #else
> > <code without __u64>
> > ..
> > #endif
> 
> This may not be possible.  You cannot account for every internal
> thing that kernel header routines might need to do or work with.
> Many structures, which the userspace can't control the layout
> of etc., makes use of the __u64 type, and we can't just turn off
> all those things just because -ansi was specified.
> 
> We're talking about things like structures that define the userspace
> ABI into the kernel, they use things like __u64.  So what effectively
> this means is that when you compile with -ansi you are effectively
> turning off access to several userspace ABIs into the kernel.
> 
> And this isn't going to be only some obscrure feature like some
> CDROM ioctl or whatever, things like "struct stat" use the 64-bit types
> either directly or indirectly.

Ok - say for instance then you were to write the abi headers - how would
you handle a case like this that -ansi forbid type long long, but it
have to be in the struct userspace uses to pass data to the
kernel/device ?


Thanks,

-- 

Martin Schlemmer




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 21:18                 ` Martin Schlemmer
@ 2003-11-06 21:18                   ` David S. Miller
  2003-11-06 21:59                     ` Martin Schlemmer
  2003-11-06 21:24                   ` Daniel Jacobowitz
  1 sibling, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-11-06 21:18 UTC (permalink / raw)
  To: azarah; +Cc: linux-kernel

On Thu, 06 Nov 2003 23:18:55 +0200
Martin Schlemmer <azarah@gentoo.org> wrote:

> Ok - say for instance then you were to write the abi headers - how would
> you handle a case like this that -ansi forbid type long long, but it
> have to be in the struct userspace uses to pass data to the
> kernel/device ?

I can tell you what cannot happen.  You absolutely can't consider
ideas like using '__u32 val[2];' and accessor macros, long long or
whatever type the platform uses for __u64 has different alignment
constraints than the '__u32 val[2]' array thing would.

I believe there is a way to work around this by using the
__extension__ keyword when defining the __u64 typedefs.  Someone
should try playing with that.  But this is only going to work when
the compiler is GCC.


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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 20:29             ` Martin Schlemmer
  2003-11-06 20:27               ` David S. Miller
@ 2003-11-06 20:40               ` H. Peter Anvin
  2003-11-06 22:31                 ` David S. Miller
  1 sibling, 1 reply; 44+ messages in thread
From: H. Peter Anvin @ 2003-11-06 20:40 UTC (permalink / raw)
  To: linux-kernel

Followup to:  <1068150552.12287.349.camel@nosferatu.lan>
By author:    Martin Schlemmer <azarah@gentoo.org>
In newsgroup: linux.dev.kernel
> 
> If you look at asm/types.h, u64 is kernel only namespace, so in
> theory that code will not be in userspace.  Also, the whole idea
> of this patch (the first one that touched byteorder.h, and not the
> second that touched types.h), was to encase everything that used
> __u64 that _is_ in userspace in __STRICT_ANSI__.  If there thus
> was another place that did use __u64 outside a ifdef __STRICT_ANSI__,
> the compile would anyhow stop with -ansi.
> 

Note that "long long" (the underlying type) is valid
standards-compliant C99.  gcc can handle it when in C89 mode if
defined as __extension__ long long IIRC.

	-hpa
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
If you send me mail in HTML format I will assume it's spam.
"Unix gives you enough rope to shoot yourself in the foot."
Architectures needed: ia64 m68k mips64 ppc ppc64 s390 s390x sh v850 x86-64

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 20:05           ` David S. Miller
@ 2003-11-06 20:29             ` Martin Schlemmer
  2003-11-06 20:27               ` David S. Miller
  2003-11-06 20:40               ` H. Peter Anvin
  2003-11-06 21:21             ` Daniel Jacobowitz
  1 sibling, 2 replies; 44+ messages in thread
From: Martin Schlemmer @ 2003-11-06 20:29 UTC (permalink / raw)
  To: David S. Miller; +Cc: KML

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

On Thu, 2003-11-06 at 22:05, David S. Miller wrote:
> On Thu, 06 Nov 2003 22:09:29 +0200
> Martin Schlemmer <azarah@gentoo.org> wrote:
> 
> > On Thu, 2003-11-06 at 21:37, David S. Miller wrote:
> > > Let's say that you end up using some inline function
> > > that takes u32 arguments, and internally it uses
> > > u64 types to speed up the calculation or make it more
> > > accurate or something like that.
> > 
> > So basically only in cases where the stuff in byteorder.h
> > was not inlined ... ?
> 
> No, exactly in the cases where it _IS_ inlined.  Imagine
> this:
> 
> static inline u32 swab_foo(u32 a, u32 b)
> {
> 	u64 tmp = ((u64)a<<32) | ((u64)b);
> 	u32 retval;
> 
> 	retval = compute(tmp);
> 
> 	return retval;
> }
> 
> If that's in a kernel header somewhere, and you build with -ansi,
> you lose.

If you look at asm/types.h, u64 is kernel only namespace, so in
theory that code will not be in userspace.  Also, the whole idea
of this patch (the first one that touched byteorder.h, and not the
second that touched types.h), was to encase everything that used
__u64 that _is_ in userspace in __STRICT_ANSI__.  If there thus
was another place that did use __u64 outside a ifdef __STRICT_ANSI__,
the compile would anyhow stop with -ansi.

Your above example would thus look more like:

--
#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
static inline __u32 swab_foo(__u32 a, __u32 b)
{
	__u64 tmp = ((__u64)a<<32) | ((__u64)b);
	__u32 retval;

	retval = compute(tmp);

	return retval;
}
#else
<code without __u64>
..
#endif
--

which in theory should not have an issue.


Thanks,

-- 

Martin Schlemmer




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 20:29             ` Martin Schlemmer
@ 2003-11-06 20:27               ` David S. Miller
  2003-11-06 21:18                 ` Martin Schlemmer
  2003-11-06 20:40               ` H. Peter Anvin
  1 sibling, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-11-06 20:27 UTC (permalink / raw)
  To: azarah; +Cc: linux-kernel

On Thu, 06 Nov 2003 22:29:12 +0200
Martin Schlemmer <azarah@gentoo.org> wrote:

> If you look at asm/types.h, u64 is kernel only namespace, so in
> theory that code will not be in userspace.

Replace u64 with __u64 in my examples, the point still stances.


> #else
> <code without __u64>
> ..
> #endif

This may not be possible.  You cannot account for every internal
thing that kernel header routines might need to do or work with.
Many structures, which the userspace can't control the layout
of etc., makes use of the __u64 type, and we can't just turn off
all those things just because -ansi was specified.

We're talking about things like structures that define the userspace
ABI into the kernel, they use things like __u64.  So what effectively
this means is that when you compile with -ansi you are effectively
turning off access to several userspace ABIs into the kernel.

And this isn't going to be only some obscrure feature like some
CDROM ioctl or whatever, things like "struct stat" use the 64-bit types
either directly or indirectly.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 19:37       ` David S. Miller
@ 2003-11-06 20:09         ` Martin Schlemmer
  2003-11-06 20:05           ` David S. Miller
  0 siblings, 1 reply; 44+ messages in thread
From: Martin Schlemmer @ 2003-11-06 20:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: KML

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

On Thu, 2003-11-06 at 21:37, David S. Miller wrote:
> On Thu, 06 Nov 2003 20:42:59 +0200
> Martin Schlemmer <azarah@gentoo.org> wrote:
> 
> > Ok, so maybe above is not a case to work on, but if I write an
> > app that use only 32bit data types, and it links to a library that
> > also handles 64bit, it does not matter, as I do not call the functions
> > that handle 64bit data types, no ?
> 
> Let's say that you end up using some inline function
> that takes u32 arguments, and internally it uses
> u64 types to speed up the calculation or make it more
> accurate or something like that.

So basically only in cases where the stuff in byteorder.h
was not inlined ... ?


Thanks,

-- 

Martin Schlemmer




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 20:09         ` Martin Schlemmer
@ 2003-11-06 20:05           ` David S. Miller
  2003-11-06 20:29             ` Martin Schlemmer
  2003-11-06 21:21             ` Daniel Jacobowitz
  0 siblings, 2 replies; 44+ messages in thread
From: David S. Miller @ 2003-11-06 20:05 UTC (permalink / raw)
  To: azarah; +Cc: linux-kernel

On Thu, 06 Nov 2003 22:09:29 +0200
Martin Schlemmer <azarah@gentoo.org> wrote:

> On Thu, 2003-11-06 at 21:37, David S. Miller wrote:
> > Let's say that you end up using some inline function
> > that takes u32 arguments, and internally it uses
> > u64 types to speed up the calculation or make it more
> > accurate or something like that.
> 
> So basically only in cases where the stuff in byteorder.h
> was not inlined ... ?

No, exactly in the cases where it _IS_ inlined.  Imagine
this:

static inline u32 swab_foo(u32 a, u32 b)
{
	u64 tmp = ((u64)a<<32) | ((u64)b);
	u32 retval;

	retval = compute(tmp);

	return retval;
}

If that's in a kernel header somewhere, and you build with -ansi,
you lose.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 18:42     ` Martin Schlemmer
@ 2003-11-06 19:37       ` David S. Miller
  2003-11-06 20:09         ` Martin Schlemmer
  0 siblings, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-11-06 19:37 UTC (permalink / raw)
  To: azarah; +Cc: linux-kernel

On Thu, 06 Nov 2003 20:42:59 +0200
Martin Schlemmer <azarah@gentoo.org> wrote:

> Ok, so maybe above is not a case to work on, but if I write an
> app that use only 32bit data types, and it links to a library that
> also handles 64bit, it does not matter, as I do not call the functions
> that handle 64bit data types, no ?

Let's say that you end up using some inline function
that takes u32 arguments, and internally it uses
u64 types to speed up the calculation or make it more
accurate or something like that.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 18:32   ` Martin Schlemmer
@ 2003-11-06 18:42     ` Martin Schlemmer
  2003-11-06 19:37       ` David S. Miller
  0 siblings, 1 reply; 44+ messages in thread
From: Martin Schlemmer @ 2003-11-06 18:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: KML

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

On Thu, 2003-11-06 at 20:32, Martin Schlemmer wrote:
> On Thu, 2003-11-06 at 19:37, David S. Miller wrote:
> > On Thu, 06 Nov 2003 19:36:39 +0200
> > Martin Schlemmer <azarah@gentoo.org> wrote:
> > 
> > > On Tue, 2003-05-06 at 04:19, David S. Miller wrote:
> > > > On Tue, 2003-05-06 at 02:16, Thomas Horsten wrote: 
> > > > > The following patch fixes the problem:
> > > >
> > > > Making the u64 swabbing functions unavailable is not an 
> > > > acceptable solution. 
> > > >
> > > 
> > > Sorry to dig this up again, but wont __STRICT_ANSI__ assume
> > > that the program will not use u64 functions (as the program/compiler
> > > is supposed to adhere to ansi standards)?
> > 
> > It may make indirect use of inline functions in the kernel headers
> > in question, which themselves need to use the u64 type.
> 
> Right, thanks.

Actually, so what ?

If we use -ansi, it means we in theory only use u16 and u32 as
data types, and if the library that was compiled with u64 support
wish to convert the u32 array/buffer pointer we pass to it to
u64, it should make sure it setup things correctly.

Ok, so maybe above is not a case to work on, but if I write an
app that use only 32bit data types, and it links to a library that
also handles 64bit, it does not matter, as I do not call the functions
that handle 64bit data types, no ?


Thanks,

-- 

Martin Schlemmer




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 17:37 ` David S. Miller
@ 2003-11-06 18:32   ` Martin Schlemmer
  2003-11-06 18:42     ` Martin Schlemmer
  0 siblings, 1 reply; 44+ messages in thread
From: Martin Schlemmer @ 2003-11-06 18:32 UTC (permalink / raw)
  To: David S. Miller; +Cc: KML

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

On Thu, 2003-11-06 at 19:37, David S. Miller wrote:
> On Thu, 06 Nov 2003 19:36:39 +0200
> Martin Schlemmer <azarah@gentoo.org> wrote:
> 
> > On Tue, 2003-05-06 at 04:19, David S. Miller wrote:
> > > On Tue, 2003-05-06 at 02:16, Thomas Horsten wrote: 
> > > > The following patch fixes the problem:
> > >
> > > Making the u64 swabbing functions unavailable is not an 
> > > acceptable solution. 
> > >
> > 
> > Sorry to dig this up again, but wont __STRICT_ANSI__ assume
> > that the program will not use u64 functions (as the program/compiler
> > is supposed to adhere to ansi standards)?
> 
> It may make indirect use of inline functions in the kernel headers
> in question, which themselves need to use the u64 type.

Right, thanks.


-- 

Martin Schlemmer



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-11-06 17:36 Martin Schlemmer
@ 2003-11-06 17:37 ` David S. Miller
  2003-11-06 18:32   ` Martin Schlemmer
  0 siblings, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-11-06 17:37 UTC (permalink / raw)
  To: azarah; +Cc: linux-kernel

On Thu, 06 Nov 2003 19:36:39 +0200
Martin Schlemmer <azarah@gentoo.org> wrote:

> On Tue, 2003-05-06 at 04:19, David S. Miller wrote:
> > On Tue, 2003-05-06 at 02:16, Thomas Horsten wrote: 
> > > The following patch fixes the problem:
> >
> > Making the u64 swabbing functions unavailable is not an 
> > acceptable solution. 
> >
> 
> Sorry to dig this up again, but wont __STRICT_ANSI__ assume
> that the program will not use u64 functions (as the program/compiler
> is supposed to adhere to ansi standards)?

It may make indirect use of inline functions in the kernel headers
in question, which themselves need to use the u64 type.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
@ 2003-11-06 17:36 Martin Schlemmer
  2003-11-06 17:37 ` David S. Miller
  0 siblings, 1 reply; 44+ messages in thread
From: Martin Schlemmer @ 2003-11-06 17:36 UTC (permalink / raw)
  To: David S. Miller; +Cc: KML

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

On Tue, 2003-05-06 at 04:19, David S. Miller wrote:
> On Tue, 2003-05-06 at 02:16, Thomas Horsten wrote: 
> > The following patch fixes the problem:
>
> Making the u64 swabbing functions unavailable is not an 
> acceptable solution. 
>

Sorry to dig this up again, but wont __STRICT_ANSI__ assume
that the program will not use u64 functions (as the program/compiler
is supposed to adhere to ansi standards)?


Thanks,

-- 

Martin Schlemmer



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  6:45                   ` Christoph Hellwig
  2003-05-07  5:44                     ` David S. Miller
@ 2003-05-07  6:59                     ` Thomas Horsten
  2003-05-07  5:53                       ` David S. Miller
  1 sibling, 1 reply; 44+ messages in thread
From: Thomas Horsten @ 2003-05-07  6:59 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: ismail (cartman) donmez, David S. Miller, marcelo, hch, linux-kernel

On Wednesday 07 May 2003 7:45 am, Christoph Hellwig wrote:
> That's highly broken because his libc was compiled against 2.2 headers.
> You must never use different headers in /usr/include/Pasm,linux} then those
> your libc was compiled against.

I don't see why moving up should be wrong - the ABI is {guaranteed | supposed} 
to remain backward compatible so the libc itself should be fine, and using 
the newer headers to build apps shouldn't hurt - at least I can't see any 
obvious cases (there are probably some, but at any rate I, have seen this 
work without problems a number of times, without rebuilding libc but using 
new features from the kernel, like iptables).

In any case, it doesn't change my example, just let Joe Admin rebuild glibc as 
well :-)

// Thomas

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  5:44                     ` David S. Miller
@ 2003-05-07  6:55                       ` Christoph Hellwig
  0 siblings, 0 replies; 44+ messages in thread
From: Christoph Hellwig @ 2003-05-07  6:55 UTC (permalink / raw)
  To: David S. Miller; +Cc: thomas, voidcartman, marcelo, linux-kernel

On Tue, May 06, 2003 at 10:44:05PM -0700, David S. Miller wrote:
>    That's highly broken because his libc was compiled against 2.2
>    headers.  You must never use different headers in
>    /usr/include/Pasm,linux} then those your libc was compiled against.
>    
> While I understand this problem, this line of reasoning simply does
> not apply for headers that libc/glibc/whatever are agnostic about.

That's how it should be.  We had tons of problems due to mismatchig
headers (usually it was "just" compile breakage because older libc
headers / compilers couldn't cope with constructs used in new kernel
headers) in the past and the only way to fix this is really don't 
ever use mismatching headers.  This is not just related to kernels,
for example Oracle ()at least up to 8i) ships .o files for their product
that were compiled on some development box but then link them at
installation time to the user's system libc.  You can guess how this
breaks :)

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  6:44                 ` Thomas Horsten
@ 2003-05-07  6:45                   ` Christoph Hellwig
  2003-05-07  5:44                     ` David S. Miller
  2003-05-07  6:59                     ` Thomas Horsten
  0 siblings, 2 replies; 44+ messages in thread
From: Christoph Hellwig @ 2003-05-07  6:45 UTC (permalink / raw)
  To: Thomas Horsten
  Cc: ismail (cartman) donmez, David S. Miller, marcelo, hch, linux-kernel

On Wed, May 07, 2003 at 07:44:27AM +0100, Thomas Horsten wrote:
> However I do not agree with that - I think it makes total sense for userland 
> to include kernel headers when we are talking e.g. specific device driver 
> interface. Imagine Joe Admin has firewall which is a pretty old Slackware 
> with 2.2 kernel and wants to upgrade to 2.4 to get from ipchains to iptables 
> (all just an example). He just downloads the 2.4 kernel and builds it, 
> symlinks to /usr/src/linux so his /usr/include/linux and ../asm will point to 
> the new kernel then he goes on to build the iptables userland binary - oops,

That's highly broken because his libc was compiled against 2.2 headers.
You must never use different headers in /usr/include/Pasm,linux} then those
your libc was compiled against.


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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  5:50               ` ismail (cartman) donmez
@ 2003-05-07  6:44                 ` Thomas Horsten
  2003-05-07  6:45                   ` Christoph Hellwig
  0 siblings, 1 reply; 44+ messages in thread
From: Thomas Horsten @ 2003-05-07  6:44 UTC (permalink / raw)
  To: ismail (cartman) donmez, David S. Miller, marcelo; +Cc: hch, linux-kernel

On Wednesday 07 May 2003 6:50 am, ismail (cartman) donmez wrote:
> On Tuesday 06 May 2003 18:40, Thomas Horsten wrote:
> > --- linux-2.4.21-rc1-orig/include/asm-i386/types.h	2002-08-03
> > 01:39:45.000000000 +0100
> > +++ linux-2.4.21-rc1-ac4/include/asm-i386/types.h	2003-05-06
> > 15:07:06.000000000 +0100
> > @@ -17,10 +17,8 @@
> >  typedef __signed__ int __s32;
> >  typedef unsigned int __u32;
> >
> > -#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
> >  typedef __signed__ long long __s64;
> >  typedef unsigned long long __u64;
> > -#endif
>
> Imho this is bad here you define a long long variable even if userspace
> apps use -ansi flag where Ansi standart has no support for long long
> variables. I think this should be fixed in userspace.

Well if you look at my earlier patch (where I changed byteorder.h instead), 
you'll see that the two places conflict for sure, and is objectively an error 
- hence the need for a fix, whatever you think about userland including these 
headers in general:

1. In types.h we define __u64 only if __STRICT_ANSI__ is *not* defined.
2. In byteorder.h we declare an inline function that uses __u64, regardless of 
whether __STRICT_ANSI__ is defined (new in 2.4.21).

It causes a compile error in the most trivial circumstance: Create a .c file 
with just one line, e.g. "include <asm/types.h>" and compile it with -ansi.

My reasoning for the original patch was: The new code added in 2.4.21 causes 
the break, __u64 is not going to be defined if -ansi is (and this has been 
the case all along, so that means with 99.9% certainty that nobody are 
actually using even the old macro version of swab64, or it would have broken 
then). So, take the inline swab64 out if __STRICT_ANSI__ is defined and don't 
use "long long".

Then David said he didn't like the approach since another header might use 
swab64 (I don't think its highly likely but it's certainly a possibility). So 
then I suggested this fix instead which he liked more. This fix is also not 
100% correct in that it breaks ANSI C, (but I'm sure other kernel headers 
might do that just as well), but at least swab64 will always be available, 
and it will work when compiling with GCC on x86 platform even with -ansi on 
(remember that the file being patched is in asm-x86 so this won't affect 
other platforms).

Another argument for the second solution is that if the userland apps are not 
supposed to include the headers in the first place then why would we ever 
check for __STRICT_ANSI__ in kernel headers.

However I do not agree with that - I think it makes total sense for userland 
to include kernel headers when we are talking e.g. specific device driver 
interface. Imagine Joe Admin has firewall which is a pretty old Slackware 
with 2.2 kernel and wants to upgrade to 2.4 to get from ipchains to iptables 
(all just an example). He just downloads the 2.4 kernel and builds it, 
symlinks to /usr/src/linux so his /usr/include/linux and ../asm will point to 
the new kernel then he goes on to build the iptables userland binary - oops, 
this didn't work if he'd relied on a separate set of kernel headers and a 
package didn't happen to be available, also what's the use of maintaining two 
sets of essentially the same header when we could just sanitize the Linux 
ones a bit (read: just enough that they don't break userland).

In summary I'm not too concerned which of the two solutions are included but I 
think one should be for sure. It's just plain wrong to have the #ifdef 
__STRICT_ANSI__ in one place and not the other.

// Thomas


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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  5:27                         ` David S. Miller
@ 2003-05-07  6:41                           ` Christoph Hellwig
  2003-05-07  5:42                             ` David S. Miller
  0 siblings, 1 reply; 44+ messages in thread
From: Christoph Hellwig @ 2003-05-07  6:41 UTC (permalink / raw)
  To: David S. Miller; +Cc: dwmw2, thomas, linux-kernel

On Tue, May 06, 2003 at 10:27:29PM -0700, David S. Miller wrote:
>    From: Christoph Hellwig <hch@infradead.org>
>    Date: Wed, 7 May 2003 07:28:30 +0100
>    
>    rtnetlink.h is a bad example.  Just to use something you quoted earlier in
>    this thread..
>    
> What is wrong with it?  Truly kernel-only elements are protected
> with __KERNEL__ the rest are only the user visible and normal
> C types that are necessary for using rtnetlink in user apps.

If we have kernel declaration in those ABI headers you'd need an updated
abi-headers package for each change in one of your prototypes, rendering
it almost useless.

For this to work you really need two classes of headers, one the defines
ABIs and only ABIs and one that's for all kernel internal stuff.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  5:19                     ` David S. Miller
@ 2003-05-07  6:28                       ` Christoph Hellwig
  2003-05-07  5:27                         ` David S. Miller
  0 siblings, 1 reply; 44+ messages in thread
From: Christoph Hellwig @ 2003-05-07  6:28 UTC (permalink / raw)
  To: David S. Miller; +Cc: dwmw2, thomas, linux-kernel

On Tue, May 06, 2003 at 10:19:00PM -0700, David S. Miller wrote:
> What about if I extend stuff without breaking the ABI?
> How do apps get at the new features?

They get the features when they use the new headers.  They ususally want
changes to support those new features anyway..

> Actually, if you look, things like include/linux/xfrm.h are excellent
> examples of userland compatible kernel headers :-)

rtnetlink.h is a bad example.  Just to use something you quoted earlier in
this thread..


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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  5:07                 ` David S. Miller
@ 2003-05-07  6:20                   ` Christoph Hellwig
  2003-05-07  5:19                     ` David S. Miller
  0 siblings, 1 reply; 44+ messages in thread
From: Christoph Hellwig @ 2003-05-07  6:20 UTC (permalink / raw)
  To: David S. Miller; +Cc: dwmw2, thomas, linux-kernel

On Tue, May 06, 2003 at 10:07:14PM -0700, David S. Miller wrote:
> This doesn't even consider the case where the ipsec-tools copy of the
> headers becomes out of date with the kernel copy.  This isn't a
> theoretical issue, this problem is real.
> 
> For example, I just changed the values of a few SADB_EALG_* values in
> pfkeyv2.h.  Now ipsec-tools is effectively broken.  Oops, when will
> the copy in ipsec-tools get updated?

You just broke the userland ABI which must not happen.  at all.  That's
why userland having older headers is fine.

> What about applications, ie. normal ones, that want to pass IPSEC
> policies into the kernel via the socket options we have that allows
> per-socket IPSEC rules to be specified?  The copy in ipsec-tools
> doesn't help them at all.

That's why we want the glibc-kernheader package.  Or even better
a package of headers that can be used by the kernel and userland,
but this would require people to properly sort out kernel header
functionality like internal structures and prototypes/inlines from
the actual ABI-relevant contents.  The networking headers currently
are very bad on this.


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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  6:59                     ` Thomas Horsten
@ 2003-05-07  5:53                       ` David S. Miller
  0 siblings, 0 replies; 44+ messages in thread
From: David S. Miller @ 2003-05-07  5:53 UTC (permalink / raw)
  To: thomas; +Cc: hch, voidcartman, marcelo, linux-kernel

   From: Thomas Horsten <thomas@horsten.com>
   Date: Wed, 7 May 2003 07:59:49 +0100

   On Wednesday 07 May 2003 7:45 am, Christoph Hellwig wrote:
   > That's highly broken because his libc was compiled against 2.2 headers.
   > You must never use different headers in /usr/include/Pasm,linux}
   > then those your libc was compiled against.
   
   I don't see why moving up should be wrong -

The headers used for 2.2.x era libc cannot cope with or conflit with
many of the constructs used by current kernel headers.

Chritoph's points are very real.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-06 15:40             ` Thomas Horsten
@ 2003-05-07  5:50               ` ismail (cartman) donmez
  2003-05-07  6:44                 ` Thomas Horsten
  0 siblings, 1 reply; 44+ messages in thread
From: ismail (cartman) donmez @ 2003-05-07  5:50 UTC (permalink / raw)
  To: Thomas Horsten, David S. Miller, marcelo; +Cc: hch, linux-kernel

On Tuesday 06 May 2003 18:40, Thomas Horsten wrote:
> --- linux-2.4.21-rc1-orig/include/asm-i386/types.h	2002-08-03
> 01:39:45.000000000 +0100
> +++ linux-2.4.21-rc1-ac4/include/asm-i386/types.h	2003-05-06
> 15:07:06.000000000 +0100
> @@ -17,10 +17,8 @@
>  typedef __signed__ int __s32;
>  typedef unsigned int __u32;
>
> -#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
>  typedef __signed__ long long __s64;
>  typedef unsigned long long __u64;
> -#endif

Imho this is bad here you define a long long variable even if userspace apps 
use -ansi flag where Ansi standart has no support for long long variables. I 
think this should be fixed in userspace.

-- 
Brain fried -- Core dumped 

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  6:45                   ` Christoph Hellwig
@ 2003-05-07  5:44                     ` David S. Miller
  2003-05-07  6:55                       ` Christoph Hellwig
  2003-05-07  6:59                     ` Thomas Horsten
  1 sibling, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-05-07  5:44 UTC (permalink / raw)
  To: hch; +Cc: thomas, voidcartman, marcelo, linux-kernel

   From: Christoph Hellwig <hch@infradead.org>
   Date: Wed, 7 May 2003 07:45:57 +0100
   
   That's highly broken because his libc was compiled against 2.2
   headers.  You must never use different headers in
   /usr/include/Pasm,linux} then those your libc was compiled against.
   
While I understand this problem, this line of reasoning simply does
not apply for headers that libc/glibc/whatever are agnostic about.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  6:41                           ` Christoph Hellwig
@ 2003-05-07  5:42                             ` David S. Miller
  0 siblings, 0 replies; 44+ messages in thread
From: David S. Miller @ 2003-05-07  5:42 UTC (permalink / raw)
  To: hch; +Cc: dwmw2, thomas, linux-kernel

   From: Christoph Hellwig <hch@infradead.org>
   Date: Wed, 7 May 2003 07:41:11 +0100
   
   If we have kernel declaration in those ABI headers you'd need an
   updated abi-headers package for each change in one of your
   prototypes, rendering it almost useless.
   
This reminds me about why this topic is actually a sticky area.

The main argument goes like this, I compile glibc against kernel
headers FOO therefore it is illegal to update headers that user apps
could see (without rebuilding GLIBC against them first) because this
could indirectly change glibc "stuff".

   For this to work you really need two classes of headers, one the defines
   ABIs and only ABIs and one that's for all kernel internal stuff.
   
I agree that this kind of splitup is desirable.  As I mentioned,
things like {linux,net}/xfrm.h are probably the best model.

Thanks for reminding me about this, I'll start to split rtnetlink.h
and friends up.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  6:28                       ` Christoph Hellwig
@ 2003-05-07  5:27                         ` David S. Miller
  2003-05-07  6:41                           ` Christoph Hellwig
  0 siblings, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-05-07  5:27 UTC (permalink / raw)
  To: hch; +Cc: dwmw2, thomas, linux-kernel

   From: Christoph Hellwig <hch@infradead.org>
   Date: Wed, 7 May 2003 07:28:30 +0100
   
   rtnetlink.h is a bad example.  Just to use something you quoted earlier in
   this thread..
   
What is wrong with it?  Truly kernel-only elements are protected
with __KERNEL__ the rest are only the user visible and normal
C types that are necessary for using rtnetlink in user apps.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  3:06             ` David S. Miller
@ 2003-05-07  5:26               ` Christoph Hellwig
  2003-05-07  5:07                 ` David S. Miller
  0 siblings, 1 reply; 44+ messages in thread
From: Christoph Hellwig @ 2003-05-07  5:26 UTC (permalink / raw)
  To: David S. Miller; +Cc: dwmw2, thomas, hch, linux-kernel

On Tue, May 06, 2003 at 08:06:38PM -0700, David S. Miller wrote:
> Listen, what do you think is the latency every time I add something
> to rtnetlink.h or pfkeyv2.h?  Should I just sit and twiddle my thumbs
> waiting for everyone to update their glibc or kernel-headers or
> whatever package before they can compile networking apps using the
> new feature?

Look at e.g. the debian and redhat packages of ipsec-tools:  they all
have their local copy of theses headers.


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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  6:20                   ` Christoph Hellwig
@ 2003-05-07  5:19                     ` David S. Miller
  2003-05-07  6:28                       ` Christoph Hellwig
  0 siblings, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-05-07  5:19 UTC (permalink / raw)
  To: hch; +Cc: dwmw2, thomas, linux-kernel

   From: Christoph Hellwig <hch@infradead.org>
   Date: Wed, 7 May 2003 07:20:02 +0100

   On Tue, May 06, 2003 at 10:07:14PM -0700, David S. Miller wrote:
   > For example, I just changed the values of a few SADB_EALG_* values in
   > pfkeyv2.h.  Now ipsec-tools is effectively broken.  Oops, when will
   > the copy in ipsec-tools get updated?
   
   You just broke the userland ABI which must not happen.  at all.  That's
   why userland having older headers is fine.
   
Wrong, the ABI in the 2.5.x IPSEC stuff is not cast in
stone yet.

What about if I extend stuff without breaking the ABI?
How do apps get at the new features?

   That's why we want the glibc-kernheader package.  Or even better
   a package of headers that can be used by the kernel and userland,
   but this would require people to properly sort out kernel header
   functionality like internal structures and prototypes/inlines from
   the actual ABI-relevant contents.  The networking headers currently
   are very bad on this.
   
Yes, this is one way to deal with it.

Actually, if you look, things like include/linux/xfrm.h are excellent
examples of userland compatible kernel headers :-)

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-07  5:26               ` Christoph Hellwig
@ 2003-05-07  5:07                 ` David S. Miller
  2003-05-07  6:20                   ` Christoph Hellwig
  0 siblings, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-05-07  5:07 UTC (permalink / raw)
  To: hch; +Cc: dwmw2, thomas, linux-kernel

   From: Christoph Hellwig <hch@infradead.org>
   Date: Wed, 7 May 2003 06:26:13 +0100
   
   Look at e.g. the debian and redhat packages of ipsec-tools:  they all
   have their local copy of theses headers.
   
You merely support my point, the situation is rediculious.

Why don't we copy headers into every app package that wants to use
certain interfaces?

#ifdef SARCASM
Yeah, that sounds like an excellent idea.
#endif /* SARCASM */

This doesn't even consider the case where the ipsec-tools copy of the
headers becomes out of date with the kernel copy.  This isn't a
theoretical issue, this problem is real.

For example, I just changed the values of a few SADB_EALG_* values in
pfkeyv2.h.  Now ipsec-tools is effectively broken.  Oops, when will
the copy in ipsec-tools get updated?

What about applications, ie. normal ones, that want to pass IPSEC
policies into the kernel via the socket options we have that allows
per-socket IPSEC rules to be specified?  The copy in ipsec-tools
doesn't help them at all.

All of this is madness, and every suggestion to copy the headers
all over the place is a non-solution.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-06 21:19           ` David Woodhouse
@ 2003-05-07  3:06             ` David S. Miller
  2003-05-07  5:26               ` Christoph Hellwig
  0 siblings, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-05-07  3:06 UTC (permalink / raw)
  To: dwmw2; +Cc: thomas, hch, linux-kernel

   From: David Woodhouse <dwmw2@infradead.org>
   Date: Tue, 06 May 2003 22:19:06 +0100

   The correct fix is to provide a userland-only version of cdrom.h which
   doesn't use the private kernel types.h. Or a file containing _only_
   those parts which can be shared between kernel and userland, defined
   using standard types such as uint32_t etc. 

Correct in your world only :-)

Listen, what do you think is the latency every time I add something
to rtnetlink.h or pfkeyv2.h?  Should I just sit and twiddle my thumbs
waiting for everyone to update their glibc or kernel-headers or
whatever package before they can compile networking apps using the
new feature?

The fact is, until that issue is solved, we have to assume that
programs are going to include kernel headers and there is nothing
we can do about it until we provide a better situation than exists
now.

Therefore, making inclusions of these kinds of headers work from
userspace is in some sense a requirement.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-06 14:10         ` Thomas Horsten
  2003-05-06 13:06           ` David S. Miller
@ 2003-05-06 21:19           ` David Woodhouse
  2003-05-07  3:06             ` David S. Miller
  1 sibling, 1 reply; 44+ messages in thread
From: David Woodhouse @ 2003-05-06 21:19 UTC (permalink / raw)
  To: Thomas Horsten; +Cc: David S. Miller, Christoph Hellwig, linux-kernel

On Tue, 2003-05-06 at 15:10, Thomas Horsten wrote:
> I see where you're coming from, but not being able to compile existing
> applications where they are never used but need to include e.g.
> cdrom.h, is IMHO even worse.

The correct fix is to provide a userland-only version of cdrom.h which
doesn't use the private kernel types.h. Or a file containing _only_
those parts which can be shared between kernel and userland, defined
using standard types such as uint32_t etc. 

-- 
dwmw2



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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-06 13:06           ` David S. Miller
@ 2003-05-06 15:40             ` Thomas Horsten
  2003-05-07  5:50               ` ismail (cartman) donmez
  0 siblings, 1 reply; 44+ messages in thread
From: Thomas Horsten @ 2003-05-06 15:40 UTC (permalink / raw)
  To: David S. Miller, marcelo; +Cc: hch, linux-kernel

On Tuesday 06 May 2003 2:06 pm, David S. Miller wrote:

>    So, would you prefer this:
>
> Yes, this looks better.

If this is then generally acceptable, Marcelo, could you please take this and 
disregard my earlier patch? I've tested this by building most of gentoo using 
those headers and it didn't seem to break anything.

Thanks :)

--- linux-2.4.21-rc1-orig/include/asm-i386/types.h	2002-08-03 
01:39:45.000000000 +0100
+++ linux-2.4.21-rc1-ac4/include/asm-i386/types.h	2003-05-06 
15:07:06.000000000 +0100
@@ -17,10 +17,8 @@
 typedef __signed__ int __s32;
 typedef unsigned int __u32;
 
-#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
 typedef __signed__ long long __s64;
 typedef unsigned long long __u64;
-#endif
 
 /*
  * These aren't exported outside the kernel to avoid name space clashes




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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-06 10:03       ` David S. Miller
@ 2003-05-06 14:10         ` Thomas Horsten
  2003-05-06 13:06           ` David S. Miller
  2003-05-06 21:19           ` David Woodhouse
  0 siblings, 2 replies; 44+ messages in thread
From: Thomas Horsten @ 2003-05-06 14:10 UTC (permalink / raw)
  To: David S. Miller, Christoph Hellwig; +Cc: linux-kernel

On Tuesday 06 May 2003 11:03 am, David S. Miller wrote:
> On Tue, 2003-05-06 at 02:49, Christoph Hellwig wrote:
> > > In any case, if the __STRICT_ANSI__ conditional is there in types.h, it
> > > should be there in byteorder.h as well.
> >
> > I don't agree with you in principle, but as this is 2.4 it's probably
> > better to just add it.  Would you mind sending Marcelo a patch?
>
> What if one of these "used from userland anyways" headers needs
> the 64-bit swabs?
>
> This is why I'm so against this patch.

I see where you're coming from, but not being able to compile existing applications 
where they are never used but need to include e.g. cdrom.h, is IMHO even worse.

This is doubly true since this breaks between 2.4.20 and 2.4.21 and the fix only 
touches the stuff that was actually changed (i.e. corrects the added inlines).

Another way would be to always define __u64 etc. in types.h, even if
__STRICT_ANSI__ is defined, given your argument that is maybe a better 
solution (why should the conditional be in types.h header if it's not meant
for userland in the first place).

That would also solve the problem (might break something else though, but
I don't think it's very likely esp. since a duplicate typedef would normally just 
be a warning).

So, would you prefer this:

--- linux-2.4.21-rc1-orig/include/asm-i386/types.h      2002-08-03 01:39:45.000000000 +0100
+++ linux-2.4.21-rc1-ac4-th/include/asm-i386/types.h       2003-05-06 15:07:06.000000000 +0100
@@ -17,10 +17,8 @@
 typedef __signed__ int __s32;
 typedef unsigned int __u32;

-#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
 typedef __signed__ long long __s64;
 typedef unsigned long long __u64;
-#endif

 /*
  * These aren't exported outside the kernel to avoid name space clashes


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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-06 14:10         ` Thomas Horsten
@ 2003-05-06 13:06           ` David S. Miller
  2003-05-06 15:40             ` Thomas Horsten
  2003-05-06 21:19           ` David Woodhouse
  1 sibling, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-05-06 13:06 UTC (permalink / raw)
  To: thomas; +Cc: hch, linux-kernel

   From: Thomas Horsten <thomas@horsten.com>
   Date: Tue, 6 May 2003 15:10:04 +0100
   
   So, would you prefer this:

Yes, this looks better.

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-06  9:49     ` Christoph Hellwig
@ 2003-05-06 10:03       ` David S. Miller
  2003-05-06 14:10         ` Thomas Horsten
  0 siblings, 1 reply; 44+ messages in thread
From: David S. Miller @ 2003-05-06 10:03 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Thomas Horsten, linux-kernel

On Tue, 2003-05-06 at 02:49, Christoph Hellwig wrote:
> > In any case, if the __STRICT_ANSI__ conditional is there in types.h, it
> > should be there in byteorder.h as well.
> 
> I don't agree with you in principle, but as this is 2.4 it's probably
> better to just add it.  Would you mind sending Marcelo a patch?

What if one of these "used from userland anyways" headers needs
the 64-bit swabs?

This is why I'm so against this patch.

-- 
David S. Miller <davem@redhat.com>

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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-06  9:47   ` Thomas Horsten
@ 2003-05-06  9:49     ` Christoph Hellwig
  2003-05-06 10:03       ` David S. Miller
  0 siblings, 1 reply; 44+ messages in thread
From: Christoph Hellwig @ 2003-05-06  9:49 UTC (permalink / raw)
  To: Thomas Horsten; +Cc: linux-kernel

On Tue, May 06, 2003 at 11:47:55AM +0200, Thomas Horsten wrote:
> What would you suggest as an alternative source for the constants in
> linux/cdrom.h when direct CD-ROM access is required (e.g. for audio
> ripping)?

A sanitzed copy of the kernel headers as e.g. in Red Hat's glibc-kerenheaders
package.  In either case cdrom.h should not need <asm/byteorder.h>

> In any case, if the __STRICT_ANSI__ conditional is there in types.h, it
> should be there in byteorder.h as well.

I don't agree with you in principle, but as this is 2.4 it's probably
better to just add it.  Would you mind sending Marcelo a patch?


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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-06  9:38 ` Christoph Hellwig
@ 2003-05-06  9:47   ` Thomas Horsten
  2003-05-06  9:49     ` Christoph Hellwig
  0 siblings, 1 reply; 44+ messages in thread
From: Thomas Horsten @ 2003-05-06  9:47 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel

On Tue, 6 May 2003, Christoph Hellwig wrote:

> > In 2.4.21-rc1 some inline functions are added to asm-i386/byteorder.h.
> > When __STRICT_ANSI__ is defined, __u64 doesn't get defined by
> > asm-i386/types.h, but it is used in one of the new inline functions,
> > __arch__swab64.
> >
> > This causes files that use __STRICT_ANSI__ and include any file that
> > relies on byteorder.h to give a compile error:
>
> It's very simple, don't include kernel headers from userland..

What would you suggest as an alternative source for the constants in
linux/cdrom.h when direct CD-ROM access is required (e.g. for audio
ripping)?

In any case, if the __STRICT_ANSI__ conditional is there in types.h, it
should be there in byteorder.h as well.

// Thomas


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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-06  9:16 Thomas Horsten
  2003-05-06  9:19 ` David S. Miller
@ 2003-05-06  9:38 ` Christoph Hellwig
  2003-05-06  9:47   ` Thomas Horsten
  1 sibling, 1 reply; 44+ messages in thread
From: Christoph Hellwig @ 2003-05-06  9:38 UTC (permalink / raw)
  To: Thomas Horsten; +Cc: linux-kernel

On Tue, May 06, 2003 at 11:16:45AM +0200, Thomas Horsten wrote:
> Hi,
> 
> In 2.4.21-rc1 some inline functions are added to asm-i386/byteorder.h.
> When __STRICT_ANSI__ is defined, __u64 doesn't get defined by
> asm-i386/types.h, but it is used in one of the new inline functions,
> __arch__swab64.
> 
> This causes files that use __STRICT_ANSI__ and include any file that
> relies on byteorder.h to give a compile error:

It's very simple, don't include kernel headers from userland..


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

* Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
  2003-05-06  9:16 Thomas Horsten
@ 2003-05-06  9:19 ` David S. Miller
  2003-05-06  9:38 ` Christoph Hellwig
  1 sibling, 0 replies; 44+ messages in thread
From: David S. Miller @ 2003-05-06  9:19 UTC (permalink / raw)
  To: Thomas Horsten; +Cc: linux-kernel

On Tue, 2003-05-06 at 02:16, Thomas Horsten wrote:
> The following patch fixes the problem:

Making the u64 swabbing functions unavailable is not an
acceptable solution.

-- 
David S. Miller <davem@redhat.com>

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

* [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)
@ 2003-05-06  9:16 Thomas Horsten
  2003-05-06  9:19 ` David S. Miller
  2003-05-06  9:38 ` Christoph Hellwig
  0 siblings, 2 replies; 44+ messages in thread
From: Thomas Horsten @ 2003-05-06  9:16 UTC (permalink / raw)
  To: linux-kernel

Hi,

In 2.4.21-rc1 some inline functions are added to asm-i386/byteorder.h.
When __STRICT_ANSI__ is defined, __u64 doesn't get defined by
asm-i386/types.h, but it is used in one of the new inline functions,
__arch__swab64.

This causes files that use __STRICT_ANSI__ and include any file that
relies on byteorder.h to give a compile error:

make[4]: Entering directory `/var/tmp/portage/kdemultimedia-3.1.1/work/kdemultimedia-3.1.1/kioslave/audiocd'
/bin/sh ../../libtool --silent --mode=compile --tag=CXX g++ -DHAVE_CONFIG_H -I. -I. -I../.. -I/usr/kde/3.1/include -I/usr/qt/3/include -I/usr/X11R6/include      -DQT_THREAD_SUPPORT  -D_REENTRANT  -Wnon-virtual-dtor -Wno-long-long -Wundef -Wall -pedantic -W -Wpointer-arith -Wmissing-prototypes -Wwrite-strings -ansi -D_XOPEN_SOURCE=500 -D_BSD_SOURCE -Wcast-align -Wconversion -DNDEBUG -DNO_DEBUG -O2 -mcpu=athlon-xp -O2 -pipe -fno-exceptions -fno-check-new -DQT_CLEAN_NAMESPACE -DQT_NO_ASCII_CAST  -c -o audiocd.lo `test -f audiocd.cpp || echo './'`audiocd.cpp
In file included from /usr/include/linux/cdrom.h:14,
                 from audiocd.cpp:57:
/usr/include/asm/byteorder.h:38: syntax error before `(' token
[.....]

The following patch fixes the problem:

--- linux-2.4.21-rc1-orig/include/asm-i386/byteorder.h	2003-05-06 09:52:33.000000000 +0100
+++ linux-2.4.21-rc1-ac4-th/include/asm-i386/byteorder.h	2003-05-06 09:51:13.000000000 +0100
@@ -34,7 +34,7 @@
 		return x;
 }

-
+#ifndef __STRICT_ANSI__
 static inline __u64 ___arch__swab64(__u64 val)
 {
 	union {
@@ -53,12 +53,17 @@
 #endif
 	return v.u;
 }
+#endif /* !__STRICT_ANSI__ */

+#ifndef __STRICT_ANSI__
 #define __arch__swab64(x) ___arch__swab64(x)
+#endif
 #define __arch__swab32(x) ___arch__swab32(x)
 #define __arch__swab16(x) ___arch__swab16(x)

+#ifndef __STRICT_ANSI__
 #define __BYTEORDER_HAS_U64__
+#endif

 #endif /* __GNUC__ */


// Thomas


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

end of thread, other threads:[~2003-11-06 23:40 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20030506110259.A29633@infradead.org>
2003-05-06 10:24 ` [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial) Thomas Horsten
2003-11-06 17:36 Martin Schlemmer
2003-11-06 17:37 ` David S. Miller
2003-11-06 18:32   ` Martin Schlemmer
2003-11-06 18:42     ` Martin Schlemmer
2003-11-06 19:37       ` David S. Miller
2003-11-06 20:09         ` Martin Schlemmer
2003-11-06 20:05           ` David S. Miller
2003-11-06 20:29             ` Martin Schlemmer
2003-11-06 20:27               ` David S. Miller
2003-11-06 21:18                 ` Martin Schlemmer
2003-11-06 21:18                   ` David S. Miller
2003-11-06 21:59                     ` Martin Schlemmer
2003-11-06 21:24                   ` Daniel Jacobowitz
2003-11-06 20:40               ` H. Peter Anvin
2003-11-06 22:31                 ` David S. Miller
2003-11-06 23:40                   ` H. Peter Anvin
2003-11-06 21:21             ` Daniel Jacobowitz
  -- strict thread matches above, loose matches on Subject: below --
2003-05-06  9:16 Thomas Horsten
2003-05-06  9:19 ` David S. Miller
2003-05-06  9:38 ` Christoph Hellwig
2003-05-06  9:47   ` Thomas Horsten
2003-05-06  9:49     ` Christoph Hellwig
2003-05-06 10:03       ` David S. Miller
2003-05-06 14:10         ` Thomas Horsten
2003-05-06 13:06           ` David S. Miller
2003-05-06 15:40             ` Thomas Horsten
2003-05-07  5:50               ` ismail (cartman) donmez
2003-05-07  6:44                 ` Thomas Horsten
2003-05-07  6:45                   ` Christoph Hellwig
2003-05-07  5:44                     ` David S. Miller
2003-05-07  6:55                       ` Christoph Hellwig
2003-05-07  6:59                     ` Thomas Horsten
2003-05-07  5:53                       ` David S. Miller
2003-05-06 21:19           ` David Woodhouse
2003-05-07  3:06             ` David S. Miller
2003-05-07  5:26               ` Christoph Hellwig
2003-05-07  5:07                 ` David S. Miller
2003-05-07  6:20                   ` Christoph Hellwig
2003-05-07  5:19                     ` David S. Miller
2003-05-07  6:28                       ` Christoph Hellwig
2003-05-07  5:27                         ` David S. Miller
2003-05-07  6:41                           ` Christoph Hellwig
2003-05-07  5:42                             ` David S. Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).