[Mesa-dev] [Mesa-stable] [PATCH] glx: Perform check for valid fbconfig against proper X-Screen.

[Emil Velikov]

On 11 October 2016 at 19:42, Mario Kleiner <mario.kleiner.de at gmail.com> wrote:
> Commit cf804b4455fac9e585b3600a8318caaced9c23de
> ('glx: fix crash with bad fbconfig') introduced a check
> in glXCreateNewContext() if the given config is a valid
> fbconfig.
>
> Unfortunately the check always checks the given config against
> the fbconfigs of the DefaultScreen(dpy), instead of the
> actual X-Screen specified in the config config->screen.
>
> This leads to failure whenever a GL context is created
> on a non-DefaultScreen(dpy), e.g., on X-Screen 1 of
> a multi-x-screen setup, where the default screen is
> typically 0.
>
> Fix this by using config->screen instead of DefaultScreen(dpy).
>
Nicely stopped Mario !

Skimming through mesa's codebase there's a few other cases of the
macro, of which the apple_glx_pbuffer_create() one (at least) needs a
similar fix.

There's also some interesting use of DefaultRootWindow(dpy), over
RootWindow(dpy, screen), plus users of the respective Xfoobar
functions rather than the foobar macros.

Can I interest you at checking/fixing those as well ;-)

Actual testing might be hard, so I'd CC Jeremy H (for the apple) and
Brian P for the xlib-libgl (src/mesa/drivers/x11/ and
src/gallium/state_trackers/glx/xlib/) ones for confirmation.

> Tested to fix context creation failure on a dual-x-screen setup.
>
> Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
> Cc: "11.2 12.0" <mesa-stable at lists.freedesktop.org>
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

> ---
>  src/glx/glxcmds.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index b0a1cb0..6abe0b9 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -1626,7 +1626,6 @@ glXCreateNewContext(Display * dpy, GLXFBConfig fbconfig,
>                      int renderType, GLXContext shareList, Bool allowDirect)
>  {
>     struct glx_config *config = (struct glx_config *) fbconfig;
> -   int screen = DefaultScreen(dpy);
>     struct glx_config **config_list;
>     int list_size;
>     unsigned i;
> @@ -1637,7 +1636,7 @@ glXCreateNewContext(Display * dpy, GLXFBConfig fbconfig,
>     }
>
>     config_list = (struct glx_config **)
> -      glXGetFBConfigs(dpy, screen, &list_size);
> +      glXGetFBConfigs(dpy, config->screen, &list_size);
>
Steven, looks like you're getting hit by this in bug 98169#comment 5.
Can you give it a test ?
Alternatively if you can track and/or open a bug report for that issue
it'll be greatly appreciated.

Thanks
Emil