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

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

[Martin Schlemmer]

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

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

[David S. Miller]

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.

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

[Martin Schlemmer]

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

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

[Martin Schlemmer]

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

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

[David S. Miller]

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.

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

[Martin Schlemmer]

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

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

[David S. Miller]

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.

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

[Martin Schlemmer]

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

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

[David S. Miller]

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.

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

[Martin Schlemmer]

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

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

[David S. Miller]

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.

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

[Martin Schlemmer]

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

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

[Daniel Jacobowitz]

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

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

[H. Peter Anvin]

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

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

[David S. Miller]

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.

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

[H. Peter Anvin]

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

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

[Daniel Jacobowitz]

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

[PATCH 2.4] byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

[Martin Schlemmer]

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

Hi Marcelo

This patch fixes include/asm-i386/types.h to define __s64 and __u64
even with -ansi passed to gcc, else we get breaks for userland that
may include include/asm-i386/byteorder.h through another header.

It is with help/comments from David S. Miller and H. Peter Anvin.


Thanks,

-- 
Martin Schlemmer

[-- Attachment #2: asm-types_h-extension.patch --]
[-- Type: text/x-patch, Size: 636 bytes --]

--- linux-2.4.22/include/asm-i386/types.h	2003-11-10 07:38:32.000000000 +0200
+++ linux-2.4.22.ext/include/asm-i386/types.h	2003-11-10 07:48:42.000000000 +0200
@@ -17,11 +17,15 @@
 typedef __signed__ int __s32;
 typedef unsigned int __u32;
 
-#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
-typedef __signed__ long long __s64;
-typedef unsigned long long __u64;
+#ifndef __GNUC__
+# ifndef __extension__
+#  define __extension__
+# endif
 #endif
 
+__extension__ typedef __signed__ long long __s64;
+__extension__ typedef unsigned long long __u64;
+
 /*
  * These aren't exported outside the kernel to avoid name space clashes
  */

[PATCH 2.6] byteorder.h breaks with __STRICT_ANSI__ defined (trivial)

[Martin Schlemmer]

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

Hi Linus

This patch fixes include/asm-i386/types.h to define __s64 and __u64
even with -ansi passed to gcc, else we get breaks for userland that
may include include/asm-i386/byteorder.h through another header.

It is with help/comments from David S. Miller and H. Peter Anvin.


Thanks,

-- 
Martin Schlemmer

[-- Attachment #2: 2.6.0-test9-asm-types_h-extension.patch --]
[-- Type: text/x-patch, Size: 559 bytes --]

--- 1/include/asm-i386/types.h	2003-11-15 15:10:09.256721568 +0200
+++ 2/include/asm-i386/types.h	2003-11-15 15:11:07.348890216 +0200
@@ -19,10 +19,14 @@
 typedef __signed__ int __s32;
 typedef unsigned int __u32;
 
-#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
-typedef __signed__ long long __s64;
-typedef unsigned long long __u64;
+#ifndef __GNUC__
+# ifndef __extension__
+#  define __extension__
+# endif
 #endif
+  
+__extension__ typedef __signed__ long long __s64;
+__extension__ typedef unsigned long long __u64;
 
 #endif /* __ASSEMBLY__ */