2003-05-06 10:12:09

by Thomas Horsten

[permalink] [raw]
Subject: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

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>


2003-05-06 14:03:44

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

From: Thomas Horsten <[email protected]>
Date: Tue, 6 May 2003 15:10:04 +0100

So, would you prefer this:

Yes, this looks better.

2003-05-06 15:27:36

by Thomas Horsten

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

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



2003-05-07 05:40:16

by ismail (cartman) donmez

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

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

2003-05-07 06:33:31

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

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.

2003-05-07 06:31:47

by Thomas Horsten

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

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

2003-05-07 06:39:13

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

From: Christoph Hellwig <[email protected]>
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.

2003-05-07 06:43:07

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

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 :)

2003-05-07 06:49:00

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

From: Thomas Horsten <[email protected]>
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.

2003-05-07 06:47:14

by Thomas Horsten

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

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

2003-11-06 17:35:53

by Martin Schlemmer

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

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



Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2003-11-06 17:43:04

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

On Thu, 06 Nov 2003 19:36:39 +0200
Martin Schlemmer <[email protected]> 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.

2003-11-06 18:31:51

by Martin Schlemmer

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

On Thu, 2003-11-06 at 19:37, David S. Miller wrote:
> On Thu, 06 Nov 2003 19:36:39 +0200
> Martin Schlemmer <[email protected]> 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



Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2003-11-06 18:42:10

by Martin Schlemmer

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

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 <[email protected]> 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




Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2003-11-06 19:42:33

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

On Thu, 06 Nov 2003 20:42:59 +0200
Martin Schlemmer <[email protected]> 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.

2003-11-06 20:08:37

by Martin Schlemmer

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

On Thu, 2003-11-06 at 21:37, David S. Miller wrote:
> On Thu, 06 Nov 2003 20:42:59 +0200
> Martin Schlemmer <[email protected]> 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




Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2003-11-06 20:11:08

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

On Thu, 06 Nov 2003 22:09:29 +0200
Martin Schlemmer <[email protected]> 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.

2003-11-06 20:28:20

by Martin Schlemmer

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

On Thu, 2003-11-06 at 22:05, David S. Miller wrote:
> On Thu, 06 Nov 2003 22:09:29 +0200
> Martin Schlemmer <[email protected]> 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




Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2003-11-06 20:32:42

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

On Thu, 06 Nov 2003 22:29:12 +0200
Martin Schlemmer <[email protected]> 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.

2003-11-06 20:41:28

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

Followup to: <[email protected]>
By author: Martin Schlemmer <[email protected]>
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
--
<[email protected]> at work, <[email protected]> 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

2003-11-06 21:18:09

by Martin Schlemmer

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

On Thu, 2003-11-06 at 22:27, David S. Miller wrote:
> On Thu, 06 Nov 2003 22:29:12 +0200
> Martin Schlemmer <[email protected]> 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




Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2003-11-06 21:24:27

by Daniel Jacobowitz

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

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 <[email protected]> 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

2003-11-06 21:24:22

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

On Thu, 06 Nov 2003 23:18:55 +0200
Martin Schlemmer <[email protected]> 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.

2003-11-06 21:21:25

by Daniel Jacobowitz

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

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 <[email protected]> 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

2003-11-06 21:31:16

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

On 6 Nov 2003 12:40:55 -0800
"H. Peter Anvin" <[email protected]> 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.

2003-11-06 21:58:29

by Martin Schlemmer

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

On Thu, 2003-11-06 at 23:18, David S. Miller wrote:
> On Thu, 06 Nov 2003 23:18:55 +0200
> Martin Schlemmer <[email protected]> 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




Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2003-11-06 23:40:58

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] 2.4.21-rc1: byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

Followup to: <[email protected]>
By author: "David S. Miller" <[email protected]>
In newsgroup: linux.dev.kernel
>
> On 6 Nov 2003 12:40:55 -0800
> "H. Peter Anvin" <[email protected]> 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


--
<[email protected]> at work, <[email protected]> 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