[Mesa-dev] [PATCH 3/4] mesa: fix typo in ATI_fs dstMod error checking

[Ian Romanick]

On 11/20/2017 04:07 PM, Miklós Máté wrote:
> Piglit: spec/ati_fragment_shader/error14-invalidmod
> 
> Signed-off-by: Miklós Máté <mtmkls at gmail.com>
> ---
>  src/mesa/main/atifragshader.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/atifragshader.c b/src/mesa/main/atifragshader.c
> index d6fc37ac8f..313ba0c66d 100644
> --- a/src/mesa/main/atifragshader.c
> +++ b/src/mesa/main/atifragshader.c
> @@ -623,7 +623,7 @@ _mesa_FragmentOpXATI(GLint optype, GLuint arg_count, GLenum op, GLuint dst,
>     }
>     if ((modtemp != GL_NONE) && (modtemp != GL_2X_BIT_ATI) &&
>        (modtemp != GL_4X_BIT_ATI) && (modtemp != GL_8X_BIT_ATI) &&
> -      (modtemp != GL_HALF_BIT_ATI) && !(modtemp != GL_QUARTER_BIT_ATI) &&
> +      (modtemp != GL_HALF_BIT_ATI) && (modtemp != GL_QUARTER_BIT_ATI) &&
>        (modtemp != GL_EIGHTH_BIT_ATI)) {
>        _mesa_error(ctx, GL_INVALID_ENUM, "C/AFragmentOpATI(dstMod)%x", modtemp);
>        return;

A good follow-up patch would be to remove all the extra () and fix the
indentation.

I'm also waffling about a simplification to the code:

    if (!util_is_power_of_two_or_zero(modtemp) ||
        modtemp > GL_EIGHTH_BIT_ATI) {
       _mesa_error(ctx, GL_INVALID_ENUM, "C/AFragmentOpATI(dstMod)%x", modtemp);
       return;
   }

util_is_power_of_two_or_zero is added by a patch series I'm about to send out.

Either way, this patch is clearly correct and is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>