[PATCH 1/1] glxstubs: Don't mix availability types on non-ELF

[Jeremy Huddleston Sequoia]

On Dec 2, 2013, at 04:37, Jon TURNEY <jon.turney at dronecode.org.uk> wrote:

> On 29/11/2013 19:36, Jeremy Huddleston Sequoia wrote:
>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu-2kanFRK1NckAvxtiuMwx3w at public.gmane.org>
>> ---
>> glx/glxstubs.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/glx/glxstubs.c b/glx/glxstubs.c
>> index 69bc004..509f65c 100644
>> --- a/glx/glxstubs.c
>> +++ b/glx/glxstubs.c
>> @@ -33,8 +33,14 @@
>> #include <inttypes.h>
>> #include "glxserver.h"
>> 
>> +#ifdef __ELF__
>> +#define _THUNK_AVAILABILITY _X_HIDDEN
>> +#else
>> +#define _THUNK_AVAILABILITY
>> +#endif
> 
> I'm not sure what bug this is fixing, but wouldn't it be better to write this
> test in a a positive form (i.e. #ifdef __MACH__) ?

See my 11/8/13 3:20p Pacific email and ajax's 11/11/13 11:06a reply (copied below).  Adam said that he "think[s] the only place where this actually matters is Xorg's libglx.so on ELF platforms".  Since the link-time fix is not in place yet, I decided to suggest just fixing up this build failure this way for now.


> +
>> #define thunk(name, type, call_args, ...) \
>> -    _X_HIDDEN void name(__VA_ARGS__) { \
>> +    _THUNK_AVAILABILITY void name(__VA_ARGS__) { \
>> 	static type proc; \
>> 	if (!proc) proc = __glGetProcAddress(#name); \
>> 	proc call_args; \
>> 
> 
> But I think there's something suspicious about this code.  I can't see
> anywhere that __glXsetGetProcAddress() gets called, so __glGetProcAddress() is
> always going to return NULL?


On Nov 11, 2013, at 11:05, Adam Jackson <ajax at redhat.com> wrote:

> On Fri, 2013-11-08 at 15:19 -0800, Jeremy Huddleston Sequoia wrote:
>> Sorry, I haven't been paying much attention to X11 for the past year
>> or so, so I missed all of this work until it started failing on my
>> build bot.
>> 
>> glxstubs.c has conflicting availability attributes.  How should we
>> resolve this?  If I was doing this for just OSX, I'd remove _X_HIDDEN
>> and just tell the linker to not export these symbols, but I'd rather
>> find a more portable solution...
> 
> Ugh, good catch.  I think fixing this at link time is probably the
> right solution.
> 
> The thing _X_HIDDEN is doing there is trying to bind the stubs to their
> callers in libglx itself.  I think the only place where this actually
> matters is Xorg's libglx.so on ELF platforms?  On servers where GLX is
> linked statically then I think normal unix binding rules mean the
> definition in the executable wins before the definition in libGL (and on
> Windows you have to explicitly import anyway).  So probably we can just
> put a version script in the linker flags for libglx and call it done.
> 
> - ajax
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4136 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20131202/b34303c4/attachment.bin>