[PATCH] headers: Fix build errors with latest glibc

[Hans de Goede]

Hi,

On 07/14/2014 03:41 PM, Daniel Stone wrote:
> Hi,
> 
> On 14 July 2014 13:51, Hans de Goede <hdegoede at redhat.com> wrote:
> 
>> On 07/14/2014 02:43 PM, Julien Cristau wrote:
>>> On Mon, Jul 14, 2014 at 14:33:00 +0200, Hans de Goede wrote:
>>>> Can I / we please get a reply from you on this ?
>>>> As explained below this is not about glamor.h, but about
>>>> making os.h safe to include which really is an
>>>> orthogonal problem, The glamor.h usage of os.h was
>>>> just a (bad) example.
>>>
>>> The drivers need to include xorg-server.h before any other server
>>> header.  AFAICT that's true both before and after your patch.
>>
> 
> Yes, this is basically my position. Including anything from <xorg/*.h>
> without having either xorg-server.h (for drivers) or xorg-config.h
> (internal) included first results in undefined behaviour. Specifically on
> 64-bit systems, you lose out horrendously because CARD32 secretly gets
> promoted via unsigned long to CARD64, as _XSERVER64 is not defined. The
> lack of all the functional definitions in xorg-server.h also means that
> your ABI's probably different even without the type differences.
> 
> It's a terrible, terrible, idea that results in really hard-to-track-down
> bug reports. To be fair, I'm not on the end of these bug reports anymore,
> but it's not fun. Anything we can do to discourage this would be better.
> 
> 
>> Well in practice they are not doing that, and they are getting
>> away with it (iow everything works just fine), except that
>> os.h starts re-defining system libc functions.
>>
> 
> 
> 
>> I really don't want to go and fix every single driver out there.
>>
> 
> To be honest, I really don't think it would be every single driver.
> 
> 
>> There may be some headers where drivers really *must* include
>> xorg-server.h first, but os.h is not one of them. So far we've
>> been getting away with os.h redefining e.g. strndup, but with
>> the latest glibc things have started to conflict.
>>
>> I know X is really really old, so we have a bunch of legacy,
>> like headers which do not work stand-alone (which is considered
>> a big no no in modern C development practices). But just because
>> we have this legacy we should no strive to do better.
>>
>> So there are 2 reasons to make these changes to os.h:
>>
>> 1) It avoids the need to fix every single driver out there
>> 2) It is simply the right thing to do
>>
> 
> os.h isn't safe to necessarily safe to include without xorg-server.h, as it
> drags in misc.h, which has: extern _X_EXPORT void SwapLongs(CARD32 *list,
> unsigned long count);
> 
> Again, CARD32 is unsigned long without xorg-server.h, which makes it 64-bit
> on those systems; including xorg-server.h defines _XSERVER64, which makes
> CARD32 actually be uint32_t. Not massively harmful, but still not really
> great. A lot of the authorization functions also use XID types, which have
> the same variance on 64-bit systems without xorg-server.h.
> 
> It might not be strictly harmful in some cases, but it's not the sort of
> thing I want to encourage. 'Always include xorg-server.h before any X
> header ever' is a really simple rule to remember for driver developers;
> muddying the water, particularly when not documented, isn't amazingly
> helpful IMHO.

Ok, fair enough. I hope someone will get around to fixup drivers where
necessary before the next Fedora mass-rebuild though. If not I guess I
will get to see how bad the driver situation wrt this really is.

Regards,

Hans