[Mesa-dev] [PATCH 2/4] mesa: fix crash when an ATI_fs pass begins with an alpha inst

[Ian Romanick]

On 11/20/2017 04:07 PM, Miklós Máté wrote:
> This fixes crash when:
> - first pass begins with alpha inst
> - first pass ends with color inst, second pass begins with alpha inst
> 
> Also, use the symbolic name instead of a number.
> Piglit: spec/ati_fragment_shader/api-alphafirst
> 
> Signed-off-by: Miklós Máté <mtmkls at gmail.com>
> ---
>  src/mesa/main/atifragshader.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/main/atifragshader.c b/src/mesa/main/atifragshader.c
> index 49ddb6e5af..d6fc37ac8f 100644
> --- a/src/mesa/main/atifragshader.c
> +++ b/src/mesa/main/atifragshader.c
> @@ -598,8 +598,10 @@ _mesa_FragmentOpXATI(GLint optype, GLuint arg_count, GLenum op, GLuint dst,
>        curProg->cur_pass=3;
>  
>     /* decide whether this is a new instruction or not ... all color instructions are new,
> -      and alpha instructions might also be new if there was no preceding color inst */
> -   if ((optype == 0) || (curProg->last_optype == optype)) {
> +      and alpha instructions might also be new if there was no preceding color inst,
> +      and this may be the first inst of the pass */

I know the code previously used this same formatting, but the Mesa style
is for the */ of a multi-line comment to be on its own line.  Each other
line should also start with a *.  And line-wrap around 78 characters.
Like:

   /* Decide whether this is a new instruction or not.  All color instructions
    * are new, and alpha instructions might also be new if there was no
    * preceding color inst.  This may also be the first inst of the pass.
    */

> +   if ((optype == ATI_FRAGMENT_SHADER_COLOR_OP) || (curProg->last_optype == optype)
> +         || (curProg->numArithInstr[curProg->cur_pass >> 1] == 0)) {

I lost the debate about where the || (or &&) should go... it should be
on the previous line.  Most of the parenthesis are unnecessary, and the
second line should line up with the opening (.

On a side topic... if anyone understands how
ati_fragment_shader::cur_pass works, it would be really great if they
could document it in mtypes.h.

>        if (curProg->numArithInstr[curProg->cur_pass >> 1] > 7) {
>  	 _mesa_error(ctx, GL_INVALID_OPERATION, "C/AFragmentOpATI(instrCount)");
>  	 return;
>