Re: [Xen-devel] [PATCH V7] xen/vm_event: Clean up control-register-write vm_events and add XCR0 event

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.05.15 at 18:32, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 05/28/2015 07:06 PM, Lengyel, Tamas wrote:
>> That seems a bit odd to me that we now have both a macro and a function
>> with the same name. I think making the macro at least all capital would
>> avoid confusion and make the code a lot more readable. But even then
>> IMHO this wrapper is not helping code readability much. Its really a
>> pain to track down auto-expanding fields in macros when someone is
>> trying to read the code.
> 
> I think the lowercase version of the wrapper macro of the same name is a
> Xen coding style convention, but if Jan thinks going uppercase is not a
> problem that's fine with me.
> 
> As for the wrapper, being right next to the function it is wrapping
> makes it hard to ignore when reading the header, and the #define is
> pretty clear on what it does.
> 
> Also, while I think that the macro does help with readability (so my
> preference would be to leave it in), it's just that - a preference - so
> if Jan also agrees that we should now remove it I'll do that. After all,

Afaic it should be kept as is.

> the macro will probably go out the window (or the first parameter will
> need to be changed to VM_EVENT_##what instead of VM_EVENT_X86_##what) as
> soon as ARM control register write events will come into play.

It's in an x86-specific header, so why should it need to be changed
for ARM? If ARM will gain a similarly named function, the use sites
will still all be architecture specific, and hence both declaration and
whether or not to have a wrapper macro can remain a per-arch
decision.

Jan


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