[Piglit] [PATCH 2/6] shader_runner: Don't call glDeleteProgram on an ARB program.

[Kenneth Graunke]

On Tuesday, November 24, 2015 02:20:10 PM Matt Turner wrote:
> On Wed, Nov 11, 2015 at 4:53 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> > On Tuesday, November 10, 2015 10:46:19 PM Matt Turner wrote:
> >> I don't feel good about this check, but it is done elsewhere in the same
> >> file ("prog == 0").
> >> ---
> >>  tests/shaders/shader_runner.c | 8 ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> >> index 4597b46..bb17381 100644
> >> --- a/tests/shaders/shader_runner.c
> >> +++ b/tests/shaders/shader_runner.c
> >> @@ -3081,8 +3081,12 @@ piglit_display(void)
> >>       if (piglit_automatic) {
> >>               free_subroutine_uniforms();
> >>               /* Free our resources, useful for valgrinding. */
> >> -             glDeleteProgram(prog);
> >> -             glUseProgram(0);
> >> +             if (prog != 0) {
> >> +                     glDeleteProgram(prog);
> >> +                     glUseProgram(0);
> >> +             } else {
> >> +                     glDeleteProgramsARB(1, &prog);
> >> +             }
> >>       }
> >>
> >>       return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> >>
> >
> > Wow.  Nasty.
> >
> > Both link_and_use_shaders() [GLSL] and compile_and_bind_program() [ARB]
> > set prog to a non-zero value.  So at first glance this seems utterly bunk.
> > However, compile_and_bind_program() creates a local "GLuint prog" variable
> > that shadows the global one, so it never actually sets this.  Heh.
> >
> > I was about to say this was correct.
> >
> > However, I don't see anything actually initializing the global prog
> > variable to 0...so won't this be using an uninitialized value?  (As
> > would the existing code you patterned this after?)
> 
> Sorry for the late reply -- I apparently didn't read your reply
> carefully enough initially.
> 
> Globals in C are automatically initialized to 0. Given that, I think
> this is okay.

Oh, right.  I think it's correct then.  Still nasty, but functional.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20151124/9634a7d8/attachment.sig>