[Mesa-dev] [PATCH] egl: Additional attribute validation for eglCreatePbufferSurface

[Ben Widawsky]

On Tue, May 17, 2016 at 06:39:02PM +0100, Plamena Manolova wrote:
> eglCreatePbufferSurface should generate an EGL_BAD_MATCH error if:
> 1: The EGL_TEXTURE_FORMAT attribute is EGL_NO_TEXTURE and EGL_TEXTURE_TARGET
> is something other than EGL_NO_TEXTURE
> 2: EGL_TEXTURE_FORMAT is something other than EGL_NO_TEXTURE and
> EGL_TEXTURE_TARGET is EGL_NO_TEXTURE.
> 
> This fixes the dEQP-EGL.functional.negative_api.create_pbuffer_surface test.
> 
> Signed-off-by: Plamena Manolova <plamena.manolova at intel.com>
> ---
>  src/egl/main/eglsurface.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
> index 2971bb0..d6b8083 100644
> --- a/src/egl/main/eglsurface.c
> +++ b/src/egl/main/eglsurface.c
> @@ -65,12 +65,14 @@ _eglClampSwapInterval(_EGLSurface *surf, EGLint interval)
>   * Parse the list of surface attributes and return the proper error code.
>   */
>  static EGLint
> -_eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint *attrib_list)
> +_eglParseSurfaceAttribList(_EGLSurface *surf, EGLint s_type, const EGLint *attrib_list)

I don't think you need to add s_type, you can simply use surf->Type which is
already defined as type.

>  {
>     _EGLDisplay *dpy = surf->Resource.Display;
>     EGLint type = surf->Type;
>     EGLint texture_type = EGL_PBUFFER_BIT;
>     EGLint i, err = EGL_SUCCESS;
> +   EGLint tex_target = -1;
> +   EGLint tex_format = -1;
>  
>     if (!attrib_list)
>        return EGL_SUCCESS;
> @@ -186,6 +188,8 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint *attrib_list)
>              err = EGL_BAD_ATTRIBUTE;
>              break;
>           }
> +
> +         tex_format = val;
>           switch (val) {
>           case EGL_TEXTURE_RGB:
>           case EGL_TEXTURE_RGBA:
> @@ -204,6 +208,8 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint *attrib_list)
>              err = EGL_BAD_ATTRIBUTE;
>              break;
>           }
> +
> +         tex_target = val;
>           switch (val) {
>           case EGL_TEXTURE_2D:
>           case EGL_NO_TEXTURE:
> @@ -229,6 +235,13 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint *attrib_list)
>           break;
>        }
>  
> +      if (s_type == EGL_PBUFFER_BIT) {
> +         if ((tex_target == EGL_NO_TEXTURE && tex_format != EGL_NO_TEXTURE) ||
> +             (tex_format == EGL_NO_TEXTURE && tex_target != EGL_NO_TEXTURE)) {
> +            err = EGL_BAD_MATCH;
> +         }
> +      }
> +

lgtm. With the s_type removed:
Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

>        if (err != EGL_SUCCESS) {
>           _eglLog(_EGL_WARNING, "bad surface attribute 0x%04x", attr);
>           break;
> @@ -303,7 +316,7 @@ _eglInitSurface(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
>     /* the default swap interval is 1 */
>     _eglClampSwapInterval(surf, 1);
>  
> -   err = _eglParseSurfaceAttribList(surf, attrib_list);
> +   err = _eglParseSurfaceAttribList(surf, type, attrib_list);
>     if (err != EGL_SUCCESS)
>        return _eglError(err, func);
>