[Mesa-dev] [PATCH] mesa: remove usage of alloca in externalobjects.c v3

[Andres Rodriguez]

On 2018-02-01 08:24 AM, Emil Velikov wrote:
> On 31 January 2018 at 19:03, Andres Rodriguez <andresx7 at gmail.com> wrote:
>> Don't want an overly large numBufferBarriers/numTextureBarriers to blow
>> up the stack.
>>
>> v2: handle malloc errors
>> v3: fix patch
>>
>> Suggested-by: Emil Velikov <emil.velikov at collabora.com>
>> Signed-off-by: Andres Rodriguez <andresx7 at gmail.com>
>> ---
>>   src/mesa/main/externalobjects.c | 48 +++++++++++++++++++++++++++++++++++------
>>   1 file changed, 42 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/main/externalobjects.c b/src/mesa/main/externalobjects.c
>> index 463debd268..a28d6dba6f 100644
>> --- a/src/mesa/main/externalobjects.c
>> +++ b/src/mesa/main/externalobjects.c
>> @@ -713,91 +713,127 @@ _mesa_WaitSemaphoreEXT(GLuint semaphore,
>>                          const GLuint *buffers,
>>                          GLuint numTextureBarriers,
>>                          const GLuint *textures,
>>                          const GLenum *srcLayouts)
>>   {
>>      GET_CURRENT_CONTEXT(ctx);
>>      struct gl_semaphore_object *semObj;
>>      struct gl_buffer_object **bufObjs;
>>      struct gl_texture_object **texObjs;
> Initialize texObjs (might as well do bufObjs) since on bufObjs
> allocation failure we'll end up feeding junk to free().
> 
>> -   bufObjs = alloca(sizeof(struct gl_buffer_object **) * numBufferBarriers);
>> +   bufObjs = malloc(sizeof(struct gl_buffer_object **) * numBufferBarriers);
>> +   if (!bufObjs) {
>> +      _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s(numBufferBarriers=%u)",
>> +                  func, numBufferBarriers);
>> +      goto end;
>> +   }
>> +
> 
>> +end:
>> +   free(bufObjs);
>> +   free(texObjs);
>>   }
>>
>>   void GLAPIENTRY
>>   _mesa_SignalSemaphoreEXT(GLuint semaphore,
>>                            GLuint numBufferBarriers,
>>                            const GLuint *buffers,
>>                            GLuint numTextureBarriers,
>>                            const GLuint *textures,
>>                            const GLenum *dstLayouts)
>>   {
>>      GET_CURRENT_CONTEXT(ctx);
>>      struct gl_semaphore_object *semObj;
>>      struct gl_buffer_object **bufObjs;
>>      struct gl_texture_object **texObjs;
>>
> Ditto.
> 
> With the above nitpicks, patch is
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

Those aren't nits, those are very real bugs. And I should've double 
checked the patch better before sending it out!

Thanks for the reviews, fixed with your feedback and pushed.

Regards,
Andres

> 
> -Emil
>