[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] Fix a BUG_ON issue




> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Tuesday, August 30, 2016 2:34 PM
> To: Wu, Feng <feng.wu@xxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxx
> Subject: RE: [PATCH] Fix a BUG_ON issue
> 
> >>> On 30.08.16 at 01:19, <feng.wu@xxxxxxxxx> wrote:
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: Monday, August 29, 2016 7:51 PM
> >> >>> On 29.08.16 at 11:14, <feng.wu@xxxxxxxxx> wrote:
> >> > --- a/xen/drivers/passthrough/io.c
> >> > +++ b/xen/drivers/passthrough/io.c
> >> > @@ -243,7 +243,7 @@ static struct vcpu *vector_hashing_dest(const
> struct
> >> domain *d,
> >> >          for ( i = 0; i <= mod; i++ )
> >> >          {
> >> >              idx = find_next_bit(dest_vcpu_bitmap, d->max_vcpus, idx) + 
> >> > 1;
> >> > -            BUG_ON(idx >= d->max_vcpus);
> >> > +            BUG_ON(idx > d->max_vcpus);
> >> >          }
> >> >
> >> >          dest = d->vcpu[idx - 1];
> >>
> >> Wouldn't it be better to change the code to
> >>
> >>         unsigned int idx = -1;
> >>
> >>         for ( i = 0; i <= mod; i++ )
> >>         {
> >>             idx = find_next_bit(dest_vcpu_bitmap, d->max_vcpus, idx + 1);
> >>             BUG_ON(idx >= d->max_vcpus);
> >>         }
> >>
> >>         dest = d->vcpu[idx];
> >
> > Thanks for the comments, both are good to me, but I slightly prefer this
> > one. Do I need to send another version?
> 
> Not necessarily - can you reason a little about your preference? I

Oh, no particular reason, just feel it looks a little simple! :) But I am fine
if you prefer the other one.

Thanks,
Feng

> particularly dislike the subtraction necessary here:
> 
>           dest = d->vcpu[idx - 1];
> 
> Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.