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

[Lofstedt, Marta]

> -----Original Message-----
> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
> Behalf Of Matt Turner
> Sent: Friday, June 26, 2015 1:40 AM
> To: Ben Widawsky
> Cc: mesa-dev at lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH v2] mesa : NULL check InfoLog
> 
> 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. :)
OK, V3 is sent I changed the title and did the indentation.

> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
----------------------------------------------------------------------
Intel Sweden AB
Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden
Registration Number: 556189-6027

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.