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

[Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx>]

On Wed Feb 18 2015 10:07:29 AM CET, Jan Beulich <JBeulich@xxxxxxxx> wrote:

> > > > 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

I do consider coding style issues relevant. Here we were talking about 
optimizing a method that is being retired in the series anyway but it is your 
call. In v6 I already made the changes you requested. 

Tamas

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