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

[Miklós Máté]

On 21/11/17 01:51, Ian Romanick wrote:
> 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>

Thanks for the review. I'll do that follow-up patch when the util 
function becomes available.

MM