[Mesa-dev] Mesa (master): mesa: add back glGetnUniform*v() overflow error reporting

[Kenneth Graunke]

On 03/15/2012 11:26 AM, Ian Romanick wrote:
> On 03/13/2012 11:35 AM, Dylan Noblesmith wrote:
>> Module: Mesa
>> Branch: master
>> Commit: b536ac6b2bc54ad9bb6e58fbd307055e419a288f
>> URL:
>> http://cgit.freedesktop.org/mesa/mesa/commit/?id=b536ac6b2bc54ad9bb6e58fbd307055e419a288f
>>
>>
>> Author: Dylan Noblesmith<nobled at dreamwidth.org>
>> Date: Thu Dec 22 21:05:38 2011 +0000
>>
>> mesa: add back glGetnUniform*v() overflow error reporting
>>
>> The error was removed in:
>>
>> commit 719909698c67c287a393d2380278e7b7495ae018
>> Author: Ian Romanick<ian.d.romanick at intel.com>
>> Date: Tue Oct 18 16:01:49 2011 -0700
>>
>> mesa: Rewrite the way uniforms are tracked and handled
>>
>> The GL_ARB_robustness spec doesn't say the implementation
>> should truncate the output, so just return after setting
>> the required error like it did before the above commit.
>
> This patch is wrong. Please revert. The ARB_robustness spec specifically
> says:
>
> "The commands
>
> void GetUniformfv(uint program, int location, float *params);
> void GetnUniformfvARB(uint program, int location, sizei bufSize,
> float *params);
> void GetUniformiv(uint program, int location, int *params);
> void GetnUniformivARB(uint program, int location, sizei bufSize,
> int *params);
> void GetUniformuiv(uint program, int location, uint *params);
> void GetnUniformuivARB(uint program, int location, sizei bufSize,
> uint *params);
> void GetUniformdv(uint program, int location, uint *params);
> void GetnUniformdvARB(uint program, int location, sizei bufSize,
> uint *params);
>
> return the value or values of the uniform at location location of
> the default uniform block for program object program in the array
> params. GetnUniformfvARB, GetnUniformivARB, GetnUniformuivARB and
> GetnUniformdvARB do not write more than <bufSize> bytes into <params>."
>
> The "do not write more than <bufSize> bytes" is the important bit.
>
> In the future, please wait for at least one Reviewed-by when changing
> API behavior, and quote spec language in the code to justify such changes.

In fairness, those patches had sat on the list for a long time, which 
seems to invoke the "silence is consent" rule.  We should have reviewed 
it earlier.