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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/03/14 11:05, Jan Beulich wrote:
>>>> On 28.03.14 at 11:55, <andrew.cooper3@xxxxxxxxxx> 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 immediately obvious is that the non-atomic variants 
>> take
>> their atomic_t at a different level of indirection to the atomic variants.
>>
>> 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, and functionally tested on x86_64
>>
>> v2: spelling fixes in commit message, and remove some redundant brackets
> I see you left even the "non-atomic atomic ops" as inline functions,
> other than suggested on v1. I'll leave it to Keir to decide whether
> the resulting larger change is worthwhile the little benefit.
>
> Jan
>

I did explain why I disagreed with leaving the non-atomic as macros. 
Any optimising compiler will generate the same code from either option,
but the static inline functions provide much more informative error
messages.

~Andrew

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