[Mesa-dev] [PATCH] i965: Add an assertion to brwProgramStringNotify.

[Kenneth Graunke]

On 07/03/2013 10:50 AM, Paul Berry wrote:
> driver->ProgramStringNotify is only called for ARB programs, fixed
> function vertex programs, and ir_to_mesa (which isn't used by the i965
> back-end).  Therefore, even after geometry shaders are added,
> brwProgramStringNotify should only ever be called with a target of
> GL_VERTEX_PROGRAM_ARB or GL_FRAGMENT_PROGRAM_ARB.
>
> This patch adds an assertion to clarify that.
> ---
>   src/mesa/drivers/dri/i965/brw_program.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c
> index 87986a9..4c5acb5 100644
> --- a/src/mesa/drivers/dri/i965/brw_program.c
> +++ b/src/mesa/drivers/dri/i965/brw_program.c
> @@ -126,7 +126,8 @@ brwProgramStringNotify(struct gl_context *ctx,
>   {
>      struct brw_context *brw = brw_context(ctx);
>
> -   if (target == GL_FRAGMENT_PROGRAM_ARB) {
> +   switch (target) {
> +   case GL_FRAGMENT_PROGRAM_ARB: {
>         struct gl_fragment_program *fprog = (struct gl_fragment_program *) prog;
>         struct brw_fragment_program *newFP = brw_fragment_program(fprog);
>         const struct brw_fragment_program *curFP =
> @@ -135,8 +136,9 @@ brwProgramStringNotify(struct gl_context *ctx,
>         if (newFP == curFP)
>   	 brw->state.dirty.brw |= BRW_NEW_FRAGMENT_PROGRAM;
>         newFP->id = get_new_program_id(brw->intel.intelScreen);
> +      break;
>      }
> -   else if (target == GL_VERTEX_PROGRAM_ARB) {
> +   case GL_VERTEX_PROGRAM_ARB: {
>         struct gl_vertex_program *vprog = (struct gl_vertex_program *) prog;
>         struct brw_vertex_program *newVP = brw_vertex_program(vprog);
>         const struct brw_vertex_program *curVP =
> @@ -152,6 +154,11 @@ brwProgramStringNotify(struct gl_context *ctx,
>         /* Also tell tnl about it:
>          */
>         _tnl_program_string(ctx, target, prog);
> +      break;
> +   }
> +   default:
> +      assert(!"Unexpected target in brwProgramStringNotify");
> +      break;
>      }

I'm really glad to see this.  However, I suspect people coming across it 
will perpetually wonder why the other targets aren't present.

Perhaps add a block comment below the "default:" containing your commit 
message?

Either way,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>