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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.02.14 at 11:26, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 24/02/14 10:02, Jan Beulich wrote:
>>>>> On 21.02.14 at 21:41, Andrew Cooper <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 immediatly obvious is that the non-atomic varients 
>>> take
>>> their atomic_t at a different level of indirection to the atomic varients.
>>>
>>> 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().
>> Did you consider leaving these "non-atomic atomic ops" untouched
>> (as they don't involve macro nesting), altering only the "real" ones?
> 
> Yes, but for the sake of three updates at callsites, I felt the benefits
> outweighed the costs.

Except that I don't really see much of a benefit here - the type safety
argument doesn't really count all that much, considering that a wrongly
used type would need to have a suitable field named "counter", which
is unlikely enough to not worry much.

Jan


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