[Mesa-stable] [Mesa-dev] [PATCH 4/8] egl: rework input validation order in _eglCreateWindowSurfaceCommon

[Tapani Pälli]

On 08/05/2017 02:25 AM, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
> 
> As mentioned in previous commit the negative tests in dEQP expect the
> arguments to be evaluated in particular order.
> 
> Namely - first the dpy, then the config, followed by the surface/window.
> 
> Move the check further down or executing the test below will produce
> the following error.
> 
>     dEQP-EGL.functional.negative_api.create_pbuffer_surface
> 
> 
>     <Section Name="Test2" Description="EGL_BAD_CONFIG is generated if config is not an EGL frame buffer configuration">
>        <Text>eglCreateWindowSurface(0x9bfff0f150, 0xffffffffffffffff, 0x0000000000000000, { EGL_NONE });</Text>
>        <Text>// 0x0000000000000000 returned</Text>
>        <Text>// ERROR expected: EGL_BAD_CONFIG, Got: EGL_BAD_NATIVE_WINDOW</Text>
>     </Section>
> 
> Cc: <mesa-stable at lists.freedesktop.org>
> Cc: Mark Janes <mark.a.janes at intel.com>
> Cc: Chad Versace <chadversary at chromium.org>
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
> Mark,
> 
> IMHO the CI does the impossible and passes the test. Perhaps it's worth
> looking into how/why it does so - I don't know.

For me the above test is passing fine on x11 (x11_egl).

> I'll pipe the series through Jenkins tomorrow - don't want to stall
> things for the guys still working.
> 
> Chad, I see that in the EGL_MESA_surfaceless implementation you
> explicitly mentioned that the surface is checked prior to the config.
> 
> Wouldn't it be better to stay consistent and move those, as per the
> above? AFAICT the spec does not explicitly dictates the order.
> ---
>   src/egl/main/eglapi.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index 3ca3dd4c7c1..3b0f896f74c 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -872,10 +872,6 @@ _eglCreateWindowSurfaceCommon(_EGLDisplay *disp, EGLConfig config,
>      _EGLSurface *surf;
>      EGLSurface ret;
>   
> -
> -   if (native_window == NULL)
> -      RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_WINDOW, EGL_NO_SURFACE);
> -
>   #ifdef HAVE_SURFACELESS_PLATFORM
>      if (disp && disp->Platform == _EGL_PLATFORM_SURFACELESS) {
>         /* From the EGL_MESA_platform_surfaceless spec (v1):
> @@ -899,6 +895,9 @@ _eglCreateWindowSurfaceCommon(_EGLDisplay *disp, EGLConfig config,
>      if ((conf->SurfaceType & EGL_WINDOW_BIT) == 0)
>         RETURN_EGL_ERROR(disp, EGL_BAD_MATCH, EGL_NO_SURFACE);
>   
> +   if (native_window == NULL)
> +      RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_WINDOW, EGL_NO_SURFACE);
> +
>      surf = drv->API.CreateWindowSurface(drv, disp, conf, native_window,
>                                          attrib_list);
>      ret = (surf) ? _eglLinkSurface(surf) : EGL_NO_SURFACE;
>