[Mesa-dev] [PATCH v2] mesa : NULL check InfoLog

[Matt Turner]

On Thu, Jun 25, 2015 at 4:16 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
> On Thu, Jun 25, 2015 at 02:52:47PM +0200, Marta Lofstedt wrote:
>> From: Marta Lofstedt <marta.lofstedt at intel.com>
>>
>> When a program is compiled, but linking failed the
>> sh->InfoLog could be NULL. This is expoloited
>> by OpenGL ES 3.1 conformance tests.
>>
>> V2: ralloc_strdup shProg->InfoLog
>>
>> Signed-off-by: Marta Lofstedt <marta.lofstedt at intel.com>
>> ---
>>  src/mesa/main/shaderapi.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> index a4296ad..bc6625a 100644
>> --- a/src/mesa/main/shaderapi.c
>> +++ b/src/mesa/main/shaderapi.c
>> @@ -1921,7 +1921,10 @@ _mesa_create_shader_program(struct gl_context* ctx, GLboolean separate,
>>  #endif
>>        }
>>
>> -      ralloc_strcat(&shProg->InfoLog, sh->InfoLog);
>> +         if (sh->InfoLog)
>> +            ralloc_strcat(&shProg->InfoLog, sh->InfoLog);
>> +         else
>> +            ralloc_strdup(ctx, shProg->InfoLog);
>
> I don't understand what the strdup part is meant to do. Without the else, this
> is:
> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>
>
> Feel free to explain why you need to dup the log in the else case, and I'll look
> again.

The story is that I misread version 1 and thought the code was not
adding anything to the log if the existing log was empty and told her
to just ralloc_strdup a new log in that case.

But as you identified, the ralloc_strdup in this patch is broken (not
using the return value), and with your suggested change we're back to
v1, so:

Marta, please add Ben's R-b to your patch as well. :)