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

Thomas Horsten
2003-05-07T06:31:47Z

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