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

[Emil Velikov]

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 ?

-Emil