Re: [Qemu-devel] [PATCH v2 1/5] bitops: Add ONES macro

[Peter Crosthwaite]

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
>