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

[Emil Velikov]

On 12 February 2016 at 02:28, Matt Turner <mattst88 at gmail.com> wrote:
> 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. :)
I was referring to "initialize it with the same data as glxcurrent.c".
Pardon if it wasn't clear enough.
Such that should current/new tests, flex a code path that relies on
the specific initial values things don't blow up.

-Emil