[Mesa-dev] [PATCH] mesa: Make GenerateMipmap check the target before finding an object.

[Brian Paul]

On 03/03/2016 03:14 AM, Kenneth Graunke wrote:
> If glGenerateMipmap was called with a bogus target, then it would
> pass that to _mesa_get_current_tex_object(), which would raise a
> _mesa_problem() telling people to file bugs.  We'd then do the
> proper error checking, raise an error, and bail.
>
> Doing the check first avoids the _mesa_problem().  The DSA variant
> doesn't take a target parameter, so we leave the target validation
> exactly as it was in that case.
>
> Fixes one dEQP GLES2 test:
> dEQP-GLES2.functional.negative_api.texture.generatemipmap.invalid_target.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>   src/mesa/main/genmipmap.c | 46 +++++++++++++++++++++++++++++-----------------
>   1 file changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/src/mesa/main/genmipmap.c b/src/mesa/main/genmipmap.c
> index 6c2d31d..0637c16 100644
> --- a/src/mesa/main/genmipmap.c
> +++ b/src/mesa/main/genmipmap.c
> @@ -38,27 +38,18 @@
>   #include "texobj.h"
>   #include "hash.h"
>
> -/**
> - * Implements glGenerateMipmap and glGenerateTextureMipmap.
> - * Generates all the mipmap levels below the base level.
> - */
> -void
> -_mesa_generate_texture_mipmap(struct gl_context *ctx,
> -                              struct gl_texture_object *texObj, GLenum target,
> -                              bool dsa)
> +static bool
> +legal_generate_mipmap_target(struct gl_context *ctx, GLenum target, bool dsa)
>   {
> -   struct gl_texture_image *srcImage;
> -   GLboolean error;
>      const char *suffix = dsa ? "Texture" : "";
> -
> -   FLUSH_VERTICES(ctx, 0);
> +   bool error;
>
>      switch (target) {
>      case GL_TEXTURE_1D:
>         error = _mesa_is_gles(ctx);
>         break;
>      case GL_TEXTURE_2D:
> -      error = GL_FALSE;
> +      error = false;
>         break;
>      case GL_TEXTURE_3D:
>         error = ctx->API == API_OPENGLES;
> @@ -70,22 +61,37 @@ _mesa_generate_texture_mipmap(struct gl_context *ctx,
>         error = _mesa_is_gles(ctx) || !ctx->Extensions.EXT_texture_array;
>         break;
>      case GL_TEXTURE_2D_ARRAY:
> -      error = (_mesa_is_gles(ctx) && ctx->Version < 30)
> -         || !ctx->Extensions.EXT_texture_array;
> +      error = (_mesa_is_gles(ctx) && ctx->Version < 30) ||
> +              !ctx->Extensions.EXT_texture_array;
>         break;
>      case GL_TEXTURE_CUBE_MAP_ARRAY:
>         error = _mesa_is_gles(ctx) ||
>                 !ctx->Extensions.ARB_texture_cube_map_array;
>         break;
>      default:
> -      error = GL_TRUE;
> +      error = true;
>      }
>
>      if (error) {
>         _mesa_error(ctx, GL_INVALID_ENUM, "glGenerate%sMipmap(target=%s)",
>                     suffix, _mesa_enum_to_string(target));
> -      return;
>      }
> +   return !error;
> +}
> +
> +/**
> + * Implements glGenerateMipmap and glGenerateTextureMipmap.
> + * Generates all the mipmap levels below the base level.
> + */
> +void
> +_mesa_generate_texture_mipmap(struct gl_context *ctx,
> +                              struct gl_texture_object *texObj, GLenum target,
> +                              bool dsa)
> +{
> +   struct gl_texture_image *srcImage;
> +   const char *suffix = dsa ? "Texture" : "";
> +
> +   FLUSH_VERTICES(ctx, 0);
>
>      if (texObj->BaseLevel >= texObj->MaxLevel) {
>         /* nothing to do */
> @@ -143,6 +149,9 @@ _mesa_GenerateMipmap(GLenum target)
>      struct gl_texture_object *texObj;
>      GET_CURRENT_CONTEXT(ctx);
>
> +   if (!legal_generate_mipmap_target(ctx, target, false))
> +      return;
> +
>      texObj = _mesa_get_current_tex_object(ctx, target);
>      if (!texObj)
>         return;
> @@ -163,5 +172,8 @@ _mesa_GenerateTextureMipmap(GLuint texture)
>      if (!texObj)
>         return;
>
> +   if (!legal_generate_mipmap_target(ctx, texObj->Target, true))
> +      return;
> +
>      _mesa_generate_texture_mipmap(ctx, texObj, texObj->Target, true);
>   }
>

Reviewed-by: Brian Paul <brianp at vmware.com>