[Mesa-dev] [PATCH v2] mesa: Deal with size differences between GLuint and GLhandleARB in GetAttachedObjectsARB

[Jeremy Sequoia]

Sent from my iPhone...

> On Jan 22, 2016, at 11:05, Ian Romanick <idr at freedesktop.org> wrote:
> 
> Please revert the version of this patch that has already landed.
> 
>> On 01/21/2016 08:48 AM, Jeremy Huddleston Sequoia wrote:
>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu at apple.com>
>> CC: Nicolai Hähnle <nhaehnle at gmail.com>
>> CC: Matt Turner <mattst88 at gmail.com>
>> ---
>> src/mesa/main/shaderapi.c | 18 +++++++++++++++++-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> index a988f41..7a0d19a 100644
>> --- a/src/mesa/main/shaderapi.c
>> +++ b/src/mesa/main/shaderapi.c
>> @@ -1374,10 +1374,26 @@ _mesa_DetachShader(GLuint program, GLuint shader)
>> 
>> void GLAPIENTRY
>> _mesa_GetAttachedObjectsARB(GLhandleARB container, GLsizei maxCount,
>> -                            GLsizei * count, GLhandleARB * obj)
>> +                            GLsizei * count, GLhandleARB * objARB)
>> {
>> +   int i;
>> +   GLuint *obj;
>> +
>>    GET_CURRENT_CONTEXT(ctx);
>> +
>> +   obj = calloc(maxCount, sizeof(GLuint));
> 
> calloc is allowed to return NULL when passed 0.  glGetAttachedObjectsARB
> is not supposed to generate an error in that condition.
> 
> If passed a negative value, glGetAttachedObjectsARB is supposed to
> generate GL_INVALID_VALUE.  In this case, since calloc will treat the
> negative value as a very large positive value, it will generate
> GL_OUT_OF_MEMORY.
> 
>> +   if (!obj) {
>> +      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderSourceARB");
>> +      return;
>> +   }
>> +
>>    get_attached_shaders(ctx, container, maxCount, count, obj);
>> +
>> +   for (i = 0 ; i < *count; i++) {
> 
> count can be NULL, so this is completely bogus.
> 
> I will submit piglit tests for all three issues.
> 
> In addition to the bugs, this patch adds almost as much code to
> _mesa_GetAttachedObjectsARB as just open-coding get_attached_shaders
> would.  If we have to do anything, I feel like that's a much better option.

Ok.  I'll revert and send a follow up that does that then.

Thanks.

> 
>> +      objARB[i] = (GLhandleARB)obj[i];
>> +   }
>> +
>> +   free(obj);
>> }
>