[PATCH] headers: Fix build errors with latest glibc

[Hans de Goede]

Hi Daniel,

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.

Regards,

Hans


On 07/03/2014 10:50 PM, Hans de Goede wrote:
> Hi Daniel,
> 
> On 07/03/2014 12:07 AM, Daniel Stone wrote:
>> Hi,
>>
>> On 2 July 2014 20:14, Hans de Goede <hdegoede at redhat.com> wrote:
>>
>>> include/os.h defines protyptes for various non ansi-c str functions, such
>>> as str[n]casecmp and strndup. The definition of the prototypes is guarded
>>> by #ifndef HAVE_STRFOO, but HAVE_STRFOO is defined by xorg-server.h which
>>> is not included by all users of os.h. E.g. glamor.h does not and should not
>>> include xorg-server.h
>>>
>>> This is a problem since the these prototype declarations clash with the
>>> libc
>>> declarations when using the latest glibc:
>>>
>>> In file included from /usr/include/xorg/misc.h:115:0,
>>>                  from /usr/include/xorg/screenint.h:50,
>>>                  from /usr/include/xorg/scrnintstr.h:50,
>>>                  from /usr/include/xorg/glamor.h:32,
>>>                  from conftest.c:61:
>>> /usr/include/xorg/os.h:579:2: error: expected identifier or '(' before
>>> '__extension__'
>>>  strndup(const char *str, size_t n);
>>>
>>> To fix this, this commit moves the HAVE_STRFOO defines to their own
>>> (generated) header called os-strfeatures.h, and unconditionally includes
>>> this
>>> header from os.h .
>>>
>>
>> NAK.
> 
> I don't understand your NAK, it seems to be purely based on glamor.h doing
> (somewhat) unrelated stuff wrong. But this fix is not for glamor.h, it is for
> os.h. The glamor.h compile error was just an example.
> 
> Currently os.h cannot be included safely (without it creating duplicate
> prototypes for system functions) without first including one of:
> dix-config.h
> xorg-server.h
> xorg-config.h
> 
> Which is IMHO quite bad for a _public_ header, esp since 2 of the 3
> headers above are not public, so the reasoning of my fix goes like
> this:
> 
> 1) This is bad, it is breaking driver builds left and right, after
> first hitting this when building the intel driver and fixing it
> at the driver level:
> http://cgit.freedesktop.org/xorg/driver/xf86-video-intel/commit/?id=40bd45fb802366aa013b9089848b67a28bf3b1ee
> I then hit the same issue a couple days later with the ati driver,
> so I decided this needs a proper fix at the server level, rather
> then fixing each and every driver out there.
> 
> 2) Since inside the server we sometime want dix-config.h and sometimes
> xorg-config.h, os.h cannot simply include xorg-server.h itself
> 
> 3) Combining 1) and 2) has let me to the conclusion that the
> relevant HAVE_STRFOO defines should be in there own header,
> which can be safely included by os.h unconditionally and installed
> as a public header.
> 
>> If glamor.h includes scrnintstr.h, it must include xorg-server.h.
>> xorg-server.h (as distinct from xorg-config.h) is the specially-generated
>> header designed explicitly to be included by all exported/SDK modules which
>> include internal headers, since it carries defines such as _XSERVER64 (and
>> many, many more) which thoroughly smash the server ABI.
>>
>> If glamor.h can't have xorg-server.h, then it can't have any other server
>> headers either.
> 
> Right, so glamor.h needs some loving, no reason to nack a patch which does
> not even touch it ...
> 
> Regards,
> 
> Hans
>