[PATCH] headers: Fix build errors with latest glibc

Hans de Goede hdegoede at redhat.com
Thu Jul 3 13:50:44 PDT 2014


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


More information about the xorg-devel mailing list