Re: [Xen-devel] [PATCH] Xen/atomic: use static inlines instead of macros

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/02/14 11:50, Ian Campbell wrote:
> On Fri, 2014-02-21 at 20:41 +0000, Andrew Cooper wrote:
>> This is some coverity-inspired tidying.
>>
>> Coverity has some grief analysing the call sites of atomic_read().  This is
>> believed to be a bug in Coverity itself when expanding the nested macros, but
>> there is no legitimate reason for it to be a macro in the first place.
>>
>> This patch changes {,_}atomic_{read,set}() from being macros to being static
>> inline functions, thus gaining some type safety.
>>
>> One issue which is not immediatly obvious is that the non-atomic varients 
>> take
>> their atomic_t at a different level of indirection to the atomic varients.
> "variants" and "immediately"

Oops - I did correct these, then sent the previous patch.

>
>> This is not suitable for _atomic_set() (when used to initialise an atomic_t)
>> which is converted to take its parameter as a pointer.  One callsite of
>> _atomic_set() is updated, while the other two callsites are updated to
>> ATOMIC_INIT().
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> CC: Keir Fraser <keir@xxxxxxx>
>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>> CC: Ian Campbell <ian.campbell@xxxxxxxxxx>
>> CC: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
>> CC: Tim Deegan <tim@xxxxxxx>
>>
>> ---
>>
>> This is compile-tested on arm32 and 64
> Thanks!
>
>> +static inline void atomic_set(atomic_t *v, int i)
>> +{
>> +    v->counter = i;
>> +}
>> +
>> +static inline void _atomic_set(atomic_t *v, int i)
>> +{
>> +    v->counter = i;
>> +}
> Are these now the same on purpose? (previously one took a pointer and
> the other the actual variable).
>
> I don't have any especially strong feelings on the patch generally. If
> x86 is going to change then I suppose ARM might as well do so for
> consistency.
>
> Ian.
>
>
>

_atomic_set(stack_var, val)  makes it a functional noop, and crucially,
leaves the caller with an untouchted atomic_t

I had to change the indirection to make x86 work, before checking arm
and finding that the two were identical.  They are used in common code
where the difference is required in x86.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel