Re: [Xen-devel] [PATCH V5 06/12] x86/hvm: factor out and rename vm_event related functions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.02.15 at 18:37, <tamas.lengyel@xxxxxxxxxxxx> wrote:
> On Tue, Feb 17, 2015 at 12:56 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> On 13.02.15 at 17:33, <tamas.lengyel@xxxxxxxxxxxx> wrote:
>>> +static void hvm_event_cr(uint32_t reason, unsigned long value,
>>> +                                unsigned long old)
>>> +{
>>> +    vm_event_request_t req = {
>>> +        .reason = reason,
>>> +        .vcpu_id = current->vcpu_id,
>>> +        .u.mov_to_cr.new_value = value,
>>> +        .u.mov_to_cr.old_value = old
>>> +    };
>>> +    uint64_t parameters = 0;
>>> +
>>> +    switch(reason)
>>
>> Coding style. Also I continue to think using switch() here rather than
>> having the caller pass both VM_EVENT_* and HVM_PARAM_* is ugly/
>> inefficient (even if the compiler may be able to sort this out for you).
> 
> It's getting retired in the series so there isn't much point in
> tweaking it here.

I realized that looking at later patches in this series, but then you
could similarly argue that the other requested adjustments are
unnecessary. But please always keep in mind that series may get
applied partially. And of course ideally a series wouldn't introduce
code just for a later patch to delete it again - i.e. if you already
find you want/need to do that, then please accept that coding
style remarks are still being made and considered relevant.

Jan


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