[Mesa-dev] [PATCH] gallium/util: fix p_atomic_dec_zero macros

[Maarten Lankhorst]

op 13-06-14 07:39, Michel Dänzer schreef:
> On 12.06.2014 17:00, Maarten Lankhorst wrote:
>> I'm pretty sure that p_atomic_dec_zero should return 1 if the count
>> drops to zero.
>>
>> Cc: "10.2 10.1 10.0" <mesa-stable at lists.freedesktop.org>
> I don't think the stable tag is justified: These bugs have been there
> for more than four years. Nothing in Gallium can work properly if the
> return value of p_atomic_dec_zero() is inverted, so if there was
> significant use of the broken variants, we should have heard about it
> long ago.
Yeah, they're just the fallbacks I guess.
>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
>> ---
>> diff --git a/src/gallium/auxiliary/util/u_atomic.h
>> b/src/gallium/auxiliary/util/u_atomic.h
>> index 2f2b42b..08cadb4 100644
>> --- a/src/gallium/auxiliary/util/u_atomic.h
>> +++ b/src/gallium/auxiliary/util/u_atomic.h
>> @@ -183,7 +183,7 @@ p_atomic_cmpxchg(int32_t *v, int32_t old, int32_t _new)
>>   
>>   #define p_atomic_set(_v, _i) (*(_v) = (_i))
>>   #define p_atomic_read(_v) (*(_v))
>> -#define p_atomic_dec_zero(_v) ((boolean) --(*(_v)))
>> +#define p_atomic_dec_zero(_v) (!(boolean) --(*(_v)))
> Will this compile (with the intended result) without another set of parens?
Compiling the following in g++ and gcc seems to confirm it works correctly:

#include <stdio.h>

int main() {
int i = 1;

printf("dropped to 0: %i\n", (!(unsigned)--(*(&i))));
return 0;
}

Output:
dropped to 0: 1

>
>> @@ -324,7 +324,7 @@ p_atomic_dec_zero(int32_t *v)
>>   {
>>      uint32_t n = atomic_dec_32_nv((uint32_t *) v);
>>   
>> -   return n != 0;
>> +   return n == 0;
>>   }
> This looks good, but there are more variants which look similarly broken?
Is this intended to be a question or a statement? The other p_atomic_dec_zero definitions look fine afaict.

~Maarten