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

Re: [Xen-devel] [PATCH] xen: clamp bitmaps to correct number of bits



>>> On 06.09.12 at 15:48, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Thu, 2012-09-06 at 13:23 +0100, Jan Beulich wrote:
>> >>> On 06.09.12 at 13:56, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
>> > +/* Ensure that the last byte is zero from nbits onwards. */
>> > +static void clamp_last_byte(uint8_t *bp, int nbits)
>> > +{
>> > +  int lastbyte = nbits/8;
>> > +  int remainder = nbits % 8;
>> > +  int mask = ((1U<<remainder)-1)&0xff;
>> 
>> While I realize the callers use plain int, I'd be very much in favor
>> of not continuing this bad practice (the more that you use 1U
>> already) - simply make the parameter and all locals (assuming
>> you really think they're useful; I would have omitted all but
>> "remainder", given they're being used just once) unsigned.
> 
> I'll fold them in, it was convenient to have variables while I was
> printk'ing what I was doing but not any more.

I won't ask you for another round because of this, but you
still left the parameter and remaining local variable as plain
int, nor did you insert whitespace into the expressions. If I
were the one to commit this, I would do the adjustment while
committing...

Anyway, as long as there's no easily visible tools side bug
addressed by this, I would think we should rather leave this
for after branching - Keir?

Jan


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


 


Rackspace

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