[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/5] bitops: Add ONES macro
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/5] bitops: Add ONES macro |
Date: |
Fri, 5 Apr 2013 19:11:20 +1000 |
Hi Peter,
On Fri, Apr 5, 2013 at 6:53 PM, Peter Maydell <address@hidden> wrote:
> On 5 April 2013 09:43, Peter Crosthwaite <address@hidden> wrote:
>> Little macro that just gives you N ones (justified to LSB).
>>
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>> ---
>>
>> include/qemu/bitops.h | 2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
>> index affcc96..da47fc8 100644
>> --- a/include/qemu/bitops.h
>> +++ b/include/qemu/bitops.h
>> @@ -273,4 +273,6 @@ static inline uint64_t deposit64(uint64_t value, int
>> start, int length,
>> return (value & ~mask) | ((fieldval << start) & mask);
>> }
>>
>> +#define ONES(num) ((num) == 64 ? ~0ull : (1ull << (num)) - 1)
>
> You can avoid the ?: here (assuming you're happy to say that
> ONES(0) is a silly thing to ask for):
>
Not that silly. ONES(0) should just stay true to the contract and
return 0. Otherwise prospective clients may have to guard against 0
cases in loops which is ugly:
for (i = 0; i < N; i++) {
....
mask = ONES(i); /* rather than ones = i ? ones(i) : 0 */
....
}
But I thought about this more, and perhaps just ditch the ? anyway,
and rely on the shift by 64 ending up as 0. Then the -1 should return
return 64 (all) ones anyway.
> #define ONES(num) (~0ULL >> (64 - (num)))
>
> Needs a documentation comment, anyway.
>
OK
Regards,
Peter
> thanks
> -- PMM
>