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

[Ian Romanick]

On 03/15/2012 01:18 PM, Kenneth Graunke wrote:
> 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.

Yeah... I'm not sure how I missed these.  I also noticed that there was 
review on the list from Brian, but that didn't end up as a Reviewed-by 
in the commit message.  So, I'll take the last part back. :)