[Mesa-dev] [PATCH v3] glx: fix error code when there is no context bound

[Tapani Pälli]

On 05/30/2016 04:35 PM, Emil Velikov wrote:
> On 30 May 2016 at 07:53, Tapani Pälli <tapani.palli at intel.com> wrote:
>> From: Bernard Kilarski <bernard.r.kilarski at intel.com>
>>
>> v2: change all related NULL checks to check against dummyContext
>> v3: really check for dummyContext *only* when ctx was from
>>     __glXGetCurrentContext
>>
>> Signed-off-by: Bernard Kilarski <bernard.r.kilarski at intel.com>
>> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> Cc: "11.2" <mesa-stable at lists.freedesktop.org>
>> ---
>>  src/glx/glx_pbuffer.c             |  2 +-
>>  src/glx/glxcmds.c                 | 12 ++++++------
>>  src/glx/query_renderer.c          |  4 ++--
>>  src/glx/tests/fake_glx_screen.cpp |  6 +++++-
>>  4 files changed, 14 insertions(+), 10 deletions(-)
>>
>
>> +++ b/src/glx/glxcmds.c
> There are a lot more places that need this fix.
>
> glXWaitGL, glXWaitX glXUseXFont glXSwapBuffers (multiple cases, split
> the apple bits?), glXGetCurrentDisplay, __glXSwapIntervalMESA,
> __glXGetSwapIntervalMESA, __glXCopySubBufferMESA
>
>
> And some more in
>  - apple/apple_xgl_api_stereo.c
> __applegl_glDrawBuffer __applegl_glDrawBuffers
>
>  - apple/apple_xgl_api_viewport.c
> __applegl_glViewport
> Might be worth splitting the Apple changes and CCing Jeremy to test/ack.
>
>  - glx_error.c
> __glXSendError
>

True, I'll add these and isolate apple specific cases in another patch.

>> +++ b/src/glx/tests/fake_glx_screen.cpp
>> @@ -75,7 +75,11 @@ indirect_create_context_attribs(struct glx_screen *base,
>>     return indirect_create_context(base, config_base, shareList, 0);
>>  }
>>
>> -__thread void *__glX_tls_Context = NULL;
>> +/* This is necessary so that we don't have to link with glxcurrent.c
>> + * which would require us to link with X libraries and what not.
>> + */
>> +struct glx_context dummyContext;
> Seemingly not required atm, although it would be safer to initialize
> it as in glxcurrent.c. Namely:
>
> struct glx_context dummyContext = {
>    &dummyBuffer[0],
>    &dummyBuffer[0],
>    &dummyBuffer[0],
>    &dummyBuffer[__GLX_BUFFER_LIMIT_SIZE],
>    sizeof(dummyBuffer),
>    &dummyVtable
> };
>
> This way things won't explode in spectacular ways as we add more tests.

I'm fine with adding dummyBuffer, dummyVtable.

>> +__thread void *__glX_tls_Context = &dummyContext;
>>
> Not 100% sure but I think we want to move this (and the dummyContext)
> in the if !defined(GLX_USE_TLS) below ?

This would not work correctly. Thing is  we need dummyContext to just 
'exist' there just to be able to build the unit tests with files that 
refer to dummyContext (but not with glxcurrent.c which results in a lot 
of trouble). Hopefully this explains the problem. There is no actual 
usage of that dummyContext.

> Regards,
> Emil
>

// Tapani