[Mesa-dev] [PATCH 01/11] mesa: Returns a GL_INVALID_VALUE error on several glGet* APIs when max length is negative

[Ian Romanick]

On 01/19/2015 03:32 AM, Eduardo Lima Mitev wrote:
> The manual page for glGetAttachedShaders, glGetShaderSource, glGetActiveUniform and
> glGetActiveUniform state that a GL_INVALID_VALUE is returned if the maximum length
> argument is less than zero. For reference, see:
> https://www.opengl.org/sdk/docs/man3/xhtml/glGetAttachedShaders.xml,
> https://www.khronos.org/opengles/sdk/docs/man31/html/glGetAttachedShaders.xhtml,
> https://www.opengl.org/sdk/docs/man3/xhtml/glGetShaderSource.xml,
> https://www.khronos.org/opengles/sdk/docs/man31/html/glGetShaderSource.xhtml,
> https://www.opengl.org/sdk/docs/man3/xhtml/glGetActiveUniform.xml,
> https://www.khronos.org/opengles/sdk/docs/man31/html/glGetActiveUniform.xhtml,
> https://www.opengl.org/sdk/docs/man3/xhtml/glGetActiveAttrib.xml,
> https://www.khronos.org/opengles/sdk/docs/man31/html/glGetActiveAttrib.xhtml.

It's probably easier to quote section 2.3.1 (Errors):

    If a negative number is provided where an argument of type sizei or
    sizeiptr is specified, an INVALID_VALUE error is generated.

I think we should be consistent (as much as possible) with the order
errors are generated.  Each function below will check things in a
different order.  get_attached_shaders can even generate multiple errors
(one in _mesa_lookup_shader_program_err and one for maxCount < 0).  If
you change all three to check max*, then call
_mesa_lookup_shader_program_err, the patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> This fixes 4 dEQP test:
> * dEQP-GLES3.functional.negative_api.state.get_attached_shaders
> * dEQP-GLES3.functional.negative_api.state.get_shader_source
> * dEQP-GLES3.functional.negative_api.state.get_active_uniform
> * dEQP-GLES3.functional.negative_api.state.get_active_attrib
> ---
>  src/mesa/main/shader_query.cpp  |  5 +++++
>  src/mesa/main/shaderapi.c       | 10 ++++++++++
>  src/mesa/main/uniform_query.cpp |  5 +++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 766ad29..df9081b 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -109,6 +109,11 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint desired_index,
>     GET_CURRENT_CONTEXT(ctx);
>     struct gl_shader_program *shProg;
>  
> +   if (maxLength < 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveAttrib(maxLength < 0)");
> +      return;
> +   }
> +
>     shProg = _mesa_lookup_shader_program_err(ctx, program, "glGetActiveAttrib");
>     if (!shProg)
>        return;
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 118e8a7..0449514 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -450,6 +450,12 @@ get_attached_shaders(struct gl_context *ctx, GLuint program, GLsizei maxCount,
>  {
>     struct gl_shader_program *shProg =
>        _mesa_lookup_shader_program_err(ctx, program, "glGetAttachedShaders");
> +
> +   if (maxCount < 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "glGetAttachedShaders(maxCount < 0)");
> +      return;
> +   }
> +
>     if (shProg) {
>        GLuint i;
>        for (i = 0; i < (GLuint) maxCount && i < shProg->NumShaders; i++) {
> @@ -786,6 +792,10 @@ get_shader_source(struct gl_context *ctx, GLuint shader, GLsizei maxLength,
>                    GLsizei *length, GLchar *sourceOut)
>  {
>     struct gl_shader *sh;
> +   if (maxLength < 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "glGetShaderSource(bufSize < 0)");
> +      return;
> +   }
>     sh = _mesa_lookup_shader_err(ctx, shader, "glGetShaderSource");
>     if (!sh) {
>        return;
> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
> index 32870d0..5cedc9c 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -51,6 +51,11 @@ _mesa_GetActiveUniform(GLuint program, GLuint index,
>     if (!shProg)
>        return;
>  
> +   if (maxLength < 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveUniform(maxLength < 0)");
> +      return;
> +   }
> +
>     if (index >= shProg->NumUserUniformStorage) {
>        _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveUniform(index)");
>        return;
>