[Mesa-dev] [PATCH] mesa: add back glGetnUniformfv() overflow error reporting

[Brian Paul]

This looks good to me.  One minor comment below.


On 03/13/2012 01:51 AM, nobled wrote:
> ping x2
>
> Also an 8.0 candidate, now that 8.0 exists.
>
> On Mon, Jan 16, 2012 at 6:52 PM, nobled<nobled at dreamwidth.org>  wrote:
>> ping?
>>
>> On Thu, Dec 22, 2011 at 4:34 PM, nobled at dreamwidth.org wrote:
>>> The error was erroneously removed in this commit:
>>>
>>> 719909698c67c287a393d2380278e7b7495ae018
>>> "mesa: Rewrite the way uniforms are tracked and handled"
>>>
>>> You also aren't even supposed to truncate the output to 'bufSize',
>>> so just return like before.
>>>
>>> Also fixup an old comment and add an assert.
>>> ---
>>> (This function has a random mixture of tabs+spaces and pure spaces for
>>> indentation, so I had no idea which style to use...)
>>>
>>>   src/mesa/main/uniform_query.cpp |   16 ++++++++++++----
>>>   src/mesa/main/uniforms.c        |    2 +-
>>>   2 files changed, 13 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
>>> index 33ba53c..8e58fc0 100644
>>> --- a/src/mesa/main/uniform_query.cpp
>>> +++ b/src/mesa/main/uniform_query.cpp
>>> @@ -203,10 +203,18 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint
>>> program, GLint location,
>>>        const union gl_constant_value *const src =
>>>          &uni->storage[offset * elements];
>>>
>>> -      unsigned bytes = sizeof(uni->storage[0]) * elements;
>>> -      if (bytes>  (unsigned) bufSize) {
>>> -        elements = bufSize / sizeof(uni->storage[0]);
>>> -        bytes = bufSize;
>>> +      assert(returnType == GLSL_TYPE_FLOAT || returnType == GLSL_TYPE_INT ||
>>> +             returnType == GLSL_TYPE_UINT);
>>> +      /* The three (currently) supported types all have the same size,
>>> +       * which is of course the same as their union. That'll change
>>> +       * with glGetUniformdv()...
>>> +       */
>>> +      unsigned bytes = sizeof(src[0]) * elements;
>>> +      if (bufSize<  0 || bytes>  (unsigned) bufSize) {
>>> +        _mesa_error( ctx, GL_INVALID_OPERATION,
>>> +                    "glGetnUniformfvARB(out of bounds: bufSize is %d,"
>>> +                    " but %u bytes are required)", bufSize, bytes );

In the error message, let's just use the generic "glGetUniform(...)" 
since this function might be called by any of the various glGetUniform 
functions (iv, if, uiv, etc).


>>> +        return;
>>>        }
>>>
>>>        /* If the return type and the uniform's native type are "compatible,"
>>> diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
>>> index 685c0f1..981874e 100644
>>> --- a/src/mesa/main/uniforms.c
>>> +++ b/src/mesa/main/uniforms.c
>>> @@ -478,7 +478,7 @@ _mesa_GetnUniformdvARB(GLhandleARB program, GLint location,
>>>     (void) params;
>>>
>>>     /*
>>> -   _mesa_get_uniform(ctx, program, location, bufSize, GL_DOUBLE, params);
>>> +   _mesa_get_uniform(ctx, program, location, bufSize,
>>> GLSL_TYPE_DOUBLE, params);
>>>     */
>>>     _mesa_error(ctx, GL_INVALID_OPERATION, "glGetUniformdvARB"
>>>                 "(GL_ARB_gpu_shader_fp64 not implemented)");
>>> --
>>> 1.7.4.1
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev