[Mesa-dev] [PATCH 3/3] mesa: Validate the target argument earlier in glGenerateMipmap()

[Laura Ekstrand]

Looks good to me.

Reviewed-by: Laura Ekstrand <laura at jlekstrand.net>

On Mon, Mar 23, 2015 at 4:30 AM, Eduardo Lima Mitev <elima at igalia.com>
wrote:

> glGenerateMipmap() queries the current texture object corresponding to the
> target
> argument before ever validating it. It does so by calling
> _mesa_get_current_tex_object() which explicitly states that target must
> have been
> validated before. However, the target validation occurs later on in
> _mesa_generate_texture_mipmap().
>
> This patch takes the target validation from
> _mesa_generate_texture_mipmap() and
> creates a new function generate_mipmap_is_valid_target(), that is called by
> both glGenerateMipmap() and glGenerateTextureMipmap().
>
> This gives the correct GL user error when an invalid target is given to
> glGenerateMipmap.
> ---
>  src/mesa/main/genmipmap.c | 52
> +++++++++++++++++++++++++++++++++--------------
>  1 file changed, 37 insertions(+), 15 deletions(-)
>
> diff --git a/src/mesa/main/genmipmap.c b/src/mesa/main/genmipmap.c
> index 9aef090..163901f 100644
> --- a/src/mesa/main/genmipmap.c
> +++ b/src/mesa/main/genmipmap.c
> @@ -39,19 +39,16 @@
>  #include "hash.h"
>
>  /**
> - * Implements glGenerateMipmap and glGenerateTextureMipmap.
> - * Generates all the mipmap levels below the base level.
> + * Validates the target argument in glGenerateMipmap and
> + * glGenerateTextureMipmap.
> + * Returns true if target is valid, otherwise returns false and sets GL
> error
> + * accordingly.
>   */
> -void
> -_mesa_generate_texture_mipmap(struct gl_context *ctx,
> -                              struct gl_texture_object *texObj, GLenum
> target,
> -                              bool dsa)
> +static bool
> +generate_mipmap_is_valid_target(struct gl_context *ctx, GLenum target,
> +                                const char *caller)
>  {
> -   struct gl_texture_image *srcImage;
> -   GLboolean error;
> -   const char *suffix = dsa ? "Texture" : "";
> -
> -   FLUSH_VERTICES(ctx, 0);
> +  bool error;
>
>     switch (target) {
>     case GL_TEXTURE_1D:
> @@ -78,15 +75,32 @@ _mesa_generate_texture_mipmap(struct gl_context *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_lookup_enum_by_nr(target));
> -      return;
> +      _mesa_error(ctx, GL_INVALID_ENUM, "%s(target=%s)",
> +                  caller, _mesa_lookup_enum_by_nr(target));
> +      return false;
>     }
>
> +   return true;
> +}
> +
> +/**
> + * 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 */
>        return;
> @@ -142,6 +156,9 @@ _mesa_GenerateMipmap(GLenum target)
>     struct gl_texture_object *texObj;
>     GET_CURRENT_CONTEXT(ctx);
>
> +   if (!generate_mipmap_is_valid_target(ctx, target, "glGenerateMipmap"))
> +      return;
> +
>     texObj = _mesa_get_current_tex_object(ctx, target);
>     if (!texObj)
>        return;
> @@ -162,5 +179,10 @@ _mesa_GenerateTextureMipmap(GLuint texture)
>     if (!texObj)
>        return;
>
> +   if (!generate_mipmap_is_valid_target(ctx, texObj->Target,
> +                                        "glGenerateTextureMipmap")) {
> +      return;
> +   }
> +
>     _mesa_generate_texture_mipmap(ctx, texObj, texObj->Target, true);
>  }
> --
> 2.1.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150323/9a271e07/attachment.html>