[Mesa-dev] [PATCH v2] egl: add check that eglCreateContext gets a valid config

[Eric Engestrom]

On Monday, 2016-10-24 07:43:26 +0300, Tapani Pälli wrote:
> 
> 
> On 10/21/2016 05:36 PM, Eric Engestrom wrote:
> > On Thursday, 2016-10-20 19:46:19 +0100, Emil Velikov wrote:
> > > On 20 October 2016 at 18:20, Tapani Pälli <tapani.palli at intel.com> wrote:
> > > > Fixes following dEQP test:
> > > > 
> > > >    dEQP-EGL.functional.negative_api.create_context
> > > > 
> > > > v2: don't break EGL_KHR_no_config_context (Eric Engestrom)
> > > > 
> > > > Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> > > > ---
> > > > 
> > > > Eric, the check you proposed does not work as KHR_no_config_context
> > > > is always there in extension list with Mesa EGL. So, solution is to
> > > > validate any given config that's not EGL_NO_CONFIG_KHR (0).
> > > > 
> > > Strictly speaking KHR_no_config_context is false for the haiku EGL
> > > driver, no idea how well though :-)
> > > 
> > > >  src/egl/main/eglapi.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> > > > index d8bd76d..ca19274 100644
> > > > --- a/src/egl/main/eglapi.c
> > > > +++ b/src/egl/main/eglapi.c
> > > > @@ -734,6 +734,9 @@ eglCreateContext(EGLDisplay dpy, EGLConfig config, EGLContext share_list,
> > > > 
> > > >     _EGL_CHECK_DISPLAY(disp, EGL_NO_CONTEXT, drv);
> > > > 
> > > > +   if (config != EGL_NO_CONFIG_KHR)
> > > > +      _EGL_CHECK_CONFIG(disp, conf, EGL_NO_CONTEXT, drv);
> > > > +
> > > >     if (!config && !disp->Extensions.KHR_no_config_context)
> > > >        RETURN_EGL_ERROR(disp, EGL_BAD_CONFIG, EGL_NO_CONTEXT);
> > > > 
> > > config != EGL_NO_CONFIG_KHR vs !config reads a bit odd - can we go for
> > > one or the other ?
> > 
> > Couldn't the second `if` be turned into an `else if(!disp->Ext...`?
> > I think that'd be clearer. (That said, it can be a separate patch.)
> > 
> > Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
> 
> I've changed it as 'else if', I will let EGL_NO_CONFIG_KHR to be there as it
> gives a better clue what's going on than !config which seems more like error
> handling than using 'configless'. Topmost patch here:
> 
> https://cgit.freedesktop.org/~tpalli/mesa/log/?h=egl_fixes

Ship it! :)

> 
> 
> > > Regardless of which one you'd go for:
> > > 
> > > Cc: "12.0 13.0" <mesa-stable at lists.freedesktop.org>
> > > Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> > > 
> > > Thanks
> > > Emil