Re: [Xen-devel] [PATCH v4 26/31] xen/x86: allow HVM guests to use hypercalls to bring up vCPUs

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.08.15 at 21:03, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 07/08/15 11:18, Roger Pau Monne wrote:
>> @@ -1135,6 +1136,161 @@ int arch_set_info_guest(
>>  #undef c
>>  }
>>  
>> +/* Called by VCPUOP_initialise for HVM guests. */
>> +static int arch_set_info_hvm_guest(struct vcpu *v, vcpu_hvm_context_t *ctx)
>> +{
>> +    struct segment_register seg;
>> +
>> +#define get_context_seg(ctx, seg, f)                                        
>> \
>> +    (ctx)->mode == VCPU_HVM_MODE_16B ? (ctx)->cpu_regs.x86_16.seg##_##f :   
>> \
>> +    (ctx)->mode == VCPU_HVM_MODE_32B ? (ctx)->cpu_regs.x86_32.seg##_##f :   
>> \
>> +    (ctx)->cpu_regs.x86_64.seg##_##f
> 
> I would be tempted to remove _context from the name, capitalise them to
> make them stick out as macros, and turn them into function style macros
> ({ }) to avoid risk of multiple expansion.

I'm kind of confused by this reply of yours: Neither does use of
({ }) make a macro function like (afaik this is tied to the macro
having parameters), nor can I see how its use would prevent
multiple expansion (even without being certain what exactly you
consider here). In general I'm advocating for not using compiler
extensions when their use doesn't serve a clear purpose. I.e. if
the above was to be re-written as a switch() statement, then the
need for using ({ }) might indeed arise.

Jan


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