[Mesa-dev] [PATCH v2] mesa : NULL check InfoLog
From: Marta Lofstedt 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 --- 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); } delete_shader(ctx, shader); -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] mesa : NULL check InfoLog
On Thu, Jun 25, 2015 at 02:52:47PM +0200, Marta Lofstedt wrote: > From: Marta Lofstedt > > 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 > --- > 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 Feel free to explain why you need to dup the log in the else case, and I'll look again. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] mesa : NULL check InfoLog
On Thu, Jun 25, 2015 at 4:16 PM, Ben Widawsky wrote: > On Thu, Jun 25, 2015 at 02:52:47PM +0200, Marta Lofstedt wrote: >> From: Marta Lofstedt >> >> 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 >> --- >> 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 > > 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. :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] mesa : NULL check InfoLog
> -Original Message- > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On > Behalf Of Matt Turner > Sent: Friday, June 26, 2015 1:40 AM > To: Ben Widawsky > Cc: mesa-dev@lists.freedesktop.org > Subject: Re: [Mesa-dev] [PATCH v2] mesa : NULL check InfoLog > > On Thu, Jun 25, 2015 at 4:16 PM, Ben Widawsky > wrote: > > On Thu, Jun 25, 2015 at 02:52:47PM +0200, Marta Lofstedt wrote: > >> From: Marta Lofstedt > >> > >> 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 > >> --- > >> 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 > > > > 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@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. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
