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

[Matt Turner]

On Thu, Feb 11, 2016 at 4:54 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 11 February 2016 at 12:04, Tapani Pälli <tapani.palli at intel.com> wrote:
>> From: Bernard Kilarski <bernard.r.kilarski at intel.com>
>>
>> Signed-off-by: Bernard Kilarski <bernard.r.kilarski at intel.com>
>> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>> Cc: "11.0 11.1" <mesa-stable at lists.freedesktop.org
>> ---
>>  src/glx/glxcmds.c                         | 2 +-
>>  src/glx/query_renderer.c                  | 4 ++--
>>  src/glx/tests/query_renderer_unittest.cpp | 4 ++++
>>  3 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
>> index 93e8db5..4db67ec 100644
>> --- a/src/glx/glxcmds.c
>> +++ b/src/glx/glxcmds.c
>> @@ -1727,7 +1727,7 @@ __glXSwapIntervalSGI(int interval)
>>     CARD32 *interval_ptr;
>>     CARD8 opcode;
>>
>> -   if (gc == NULL) {
>> +   if (gc == NULL || gc == &dummyContext) {
>>        return GLX_BAD_CONTEXT;
>>     }
>>
>> diff --git a/src/glx/query_renderer.c b/src/glx/query_renderer.c
>> index 9108ec2..d49b8fe 100644
>> --- a/src/glx/query_renderer.c
>> +++ b/src/glx/query_renderer.c
>> @@ -106,7 +106,7 @@ glXQueryCurrentRendererIntegerMESA(int attribute, unsigned int *value)
>>  {
>>     struct glx_context *gc = __glXGetCurrentContext();
>>
>> -   if (gc == NULL)
>> +   if (gc == NULL || gc == &dummyContext)
>>        return False;
>>
>>     return __glXQueryRendererInteger(gc->psc, attribute, value);
>> @@ -166,7 +166,7 @@ glXQueryCurrentRendererStringMESA(int attribute)
>>  {
>>     struct glx_context *gc = __glXGetCurrentContext();
>>
>> -   if (gc == NULL)
>> +   if (gc == NULL || gc == &dummyContext)
> Unless I'm missing something __glXGetCurrentContext() can never return
> NULL, right ?
>
> In that case I see two possible solutions
>  - Swap all the NULL checks,  for == &dummyContext (some commit
> message would be great) or
>  - Drop the null checks in this patch, and explain why we do that only
> here (the xid and vtable are implicitly zeroed for dummyContext, thus
> things will just work).
>
> Leaning for the latter for stable as it's shorter. Although doing the
> former on top (for master alone) also sounds like a good idea imho.
>
>>        return False;
>>
>>     return __glXQueryRendererString(gc->psc, attribute);
>> diff --git a/src/glx/tests/query_renderer_unittest.cpp b/src/glx/tests/query_renderer_unittest.cpp
>> index 2f3c4ef..4c96260 100644
>> --- a/src/glx/tests/query_renderer_unittest.cpp
>> +++ b/src/glx/tests/query_renderer_unittest.cpp
>> @@ -40,6 +40,10 @@ struct attribute_test_vector {
>>  #define E(x) { # x, x }
>>
>>
>> +/* 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;
>>
> Perhaps we should initialize it (like in glxcurrent.c) for
> consistency, if anything else ?

Global variables are initialized. :)