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
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..
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?
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
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 <[email protected]>
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 <[email protected]>
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
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
From: David Woodhouse <[email protected]>
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.
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.
From: Christoph Hellwig <[email protected]>
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.
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.
From: Christoph Hellwig <[email protected]>
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 :-)
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..
From: Christoph Hellwig <[email protected]>
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.
On Tue, May 06, 2003 at 10:27:29PM -0700, David S. Miller wrote:
> From: Christoph Hellwig <[email protected]>
> 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.
From: Christoph Hellwig <[email protected]>
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.