Re: [PATCH 02/16] x86: fix: make atomic_read() param const

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 12/07/16 11:38, Corneliu ZUZU wrote:
> On 7/12/2016 1:22 PM, Andrew Cooper wrote:
>> On 12/07/16 11:11, Corneliu ZUZU wrote:
>>>>>> If you are really feeling up to it, having a common xen/atomic.h
>>>>>> with
>>>>>>
>>>>>> typedef struct { int counter; } atomic_t;
>>>>>> #define ATOMIC_INIT(i) { (i) }
>>>>>>
>>>>>> and some prototypes such as:
>>>>>>
>>>>>> static inline int atomic_read(const atomic_t *v);
>>>>>>
>>>>>> would be great, but this looks like it has the possibility to turn
>>>>>> into
>>>>>> a rats nest.  If it does, then just doubling up this code for arm
>>>>>> is ok.
>>>>>>
>>>>>> ~Andrew
>>>>> Yes, that might be more complicated than we expect and I don't
>>>>> know if
>>>>> making code such as this common would be a good idea, usually these
>>>>> functions are always architecture-specific.
>>>> I only suggested making the prototype common, not the implementation.
>>>> As such, the issue you accidentally introduced would become a hard
>>>> build
>>>> failure on affected architectures, rather than a subtle build
>>>> failure in
>>>> common code at some point in the future.
>>>>
>>>> ~Andrew
>>>>
>>> Oh, I see, good idea, I've just tested it and it works, what did you
>>> have in mind when you said it could cause problems?
>> The build issues would come at some point later when someone attempts to
>> atomic_read() a constant atomic_t in common code, when the ARM build
>> would break.
>>
>> ~Andrew
>
> Ooh, no, I was asking what you meant when you said "this looks like it
> has the possibility to turn into a rats nest" in your first message,
> not the thing about hard build failure..

Ah. sorry.

You would have to invert all the includes of atomic.h to include
<xen/atomic.h> rather than <asm/atomic.h>, and have xen/atomic.h include
asm/atomic.h towards the end, such that the common prototypes are
first.  I just suspect that this might not be completely trivial to
untangle (of course, I could also be wrong).

~Andrew

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