[Mesa-dev] [PATCH] egl: Fix the bad surface attributes combination checking for pbuffers.

[Frank Binns]

On 18/06/16 01:34, Guillaume Charifi wrote:
> Fixes a regression induced by commit a0674ce5:
You should put the full hash as this shortened version won't necessarily be
unique in the future.

Also, one minor comment below.

With the hash fixed this is:
Reviewed-by: Frank Binns <frank.binns at imgtec.com>

> When EGL_TEXTURE_FORMAT and EGL_TEXTURE_TARGET were both specified (and
> both != EGL_NO_TEXTURE), an error was instantly triggered, before the
> other one had even a chance to be checked, which is obviously not the
> intended behaviour.
>
> Signed-off-by: Guillaume Charifi <guillaume.charifi at sfr.fr>
> ---
>   src/egl/main/eglsurface.c | 34 +++++++++++++++++++---------------
>   1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
> index 99e24dd..61e7d47 100644
> --- a/src/egl/main/eglsurface.c
> +++ b/src/egl/main/eglsurface.c
> @@ -73,6 +73,8 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint *attrib_list)
>      EGLint i, err = EGL_SUCCESS;
>      EGLint tex_target = -1;
>      EGLint tex_format = -1;
> +   EGLint attr = EGL_NONE;
> +   EGLint val = EGL_NONE;
>   
>      if (!attrib_list)
>         return EGL_SUCCESS;
> @@ -81,8 +83,8 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint *attrib_list)
>         texture_type |= EGL_PIXMAP_BIT;
>   
>      for (i = 0; attrib_list[i] != EGL_NONE; i++) {
> -      EGLint attr = attrib_list[i++];
> -      EGLint val = attrib_list[i];
> +      attr = attrib_list[i++];
> +      val = attrib_list[i];
>   
>         switch (attr) {
>         /* common attributes */
> @@ -235,25 +237,27 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint *attrib_list)
>            break;
>         }
>   
> -      if (type == EGL_PBUFFER_BIT) {
> -         if (tex_target == -1)
> -            tex_target = surf->TextureTarget;
> +      if (err != EGL_SUCCESS)
> +         break;
> +   }
>   
> -         if (tex_format == -1)
> -            tex_format = surf->TextureFormat;
> +   if (err == EGL_SUCCESS && type == EGL_PBUFFER_BIT) {
> +      if (tex_target == -1)
> +         tex_target = surf->TextureTarget;
>   
> -         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;
> -         }
> -      }
> +      if (tex_format == -1)
> +         tex_format = surf->TextureFormat;
tex_target and tex_format seem unnecessary as you can just test against
surf->TextureTarget and surf->TextureFormat directly. If you do want to 
continue
using them then, you can simplify things by declaring them inside this 
block and
setting them unconditionally.

>   
> -      if (err != EGL_SUCCESS) {
> -         _eglLog(_EGL_WARNING, "bad surface attribute 0x%04x", attr);
> -         break;
> +      if ((tex_target == EGL_NO_TEXTURE && tex_format != EGL_NO_TEXTURE) ||
> +          (tex_format == EGL_NO_TEXTURE && tex_target != EGL_NO_TEXTURE)) {
> +         attr = tex_target == EGL_NO_TEXTURE ? EGL_TEXTURE_TARGET : EGL_TEXTURE_FORMAT;
> +         err = EGL_BAD_MATCH;
>         }
>      }
>   
> +   if (err != EGL_SUCCESS)
> +      _eglLog(_EGL_WARNING, "bad surface attribute 0x%04x", attr);
> +
>      return err;
>   }
>