Re: [Qemu-devel] [PATCH 01/38] bitmap: use long as index

[Stefan Weil]

Am 17.12.2013 19:05, schrieb Eric Blake:
> On 12/17/2013 08:25 AM, Juan Quintela wrote:
>> Move index and size fields from int to long.  We need that for
>> migration.  long is 64 bits on sane architectures, and 32bits should
>> be enough on all the 32bits architectures.


Does this also work for "insane" architectures like Windows (64 bit) where
long is only 32 bit? Wouldn't uintptr_t or intptr_t be better (also for
x32)?


>>
>>
>> Signed-off-by: Juan Quintela <address@hidden>
>> ---
>>  include/qemu/bitmap.h | 77
++++++++++++++++++++++++++-------------------------
>>  include/qemu/bitops.h | 14 +++++-----
>>  util/bitmap.c         | 60 +++++++++++++++++++--------------------
>>  3 files changed, 76 insertions(+), 75 deletions(-)
>>
>> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
>> index 308bbb7..afdd257 100644
>> --- a/include/qemu/bitmap.h
>> +++ b/include/qemu/bitmap.h
>> @@ -31,7 +31,7 @@
>>   * bitmap_andnot(dst, src1, src2, nbits)    *dst = *src1 & ~(*src2)
>>   * bitmap_complement(dst, src, nbits)        *dst = ~(*src)
>>   * bitmap_equal(src1, src2, nbits)        Are *src1 and *src2 equal?
>> - * bitmap_intersects(src1, src2, nbits)     Do *src1 and *src2 overlap?
>> + * bitmap_intersects(src1, src2, nbits)         Do *src1 and *src2
overlap?
>
> Spurious whitespace change?
>
>>   * bitmap_empty(src, nbits)            Are all bits zero in *src?
>>   * bitmap_full(src, nbits)            Are all bits set in *src?
>>   * bitmap_set(dst, pos, nbits)            Set specified bit area
>> @@ -62,71 +62,71 @@
>>          )
>>
>>  #define DECLARE_BITMAP(name,bits)                  \
>> -    unsigned long name[BITS_TO_LONGS(bits)]
>> +        unsigned long name[BITS_TO_LONGS(bits)]
>>
>>  #define small_nbits(nbits)                      \
>> -    ((nbits) <= BITS_PER_LONG)
>> +        ((nbits) <= BITS_PER_LONG)
>
> Whitespace change, but in same hunk as real changes, so ok for
> checkpatch.pl reasons.
>
>> +++ b/include/qemu/bitops.h
>> @@ -28,7 +28,7 @@
>>   * @nr: the bit to set
>>   * @addr: the address to start counting from
>>   */
>> -static inline void set_bit(int nr, unsigned long *addr)
>> +static inline void set_bit(long nr, unsigned long *addr)
>>  {
>>      unsigned long mask = BIT_MASK(nr);
>>          unsigned long *p = addr + BIT_WORD(nr);
>
> Worth cleaning up this whitespace while in the area?
>
> Content changes seem sane to me:
> Reviewed-by: Eric Blake <address@hidden>
>

Cheers,
Stefan