Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

Joe Perches
2013-10-17T18:13:35Z

On Thu, 2013-10-17 at 09:10 -0700, Tony Luck wrote:
> On Thu, Oct 17, 2013 at 1:55 AM, Joe Perches <joe@perches.com> wrote:
> > It's cost free to add the BUILD_BUG_ON
> > and perhaps you underestimate the runtime
> > bug checking effort,
>
> This looks OK to me. Gong: This doesn't stop people from using variables
> as arguments ... they just won't get a check for (h) < (l).

Another possibility is to swap high and low if necessary and
maybe warn when either is negative or too large for the bit width.

#define GENMASK(h, l) \
({ \
size_t high = h; \
size_t low = l; \
BUILD_BUG_ON(__builtin_constant_p(l) && \
__builtin_constant_p(h) && \
(l) > (h)); \
BUILD_BUG_ON(__builtin_constant_p(l) && \
__builtin_constant_p(h) && \
((l) >= BITS_PER_LONG || \
(h) >= BITS_PER_LONG)); \
WARN_ONCE((!__builtin_constant_p(l) && \
((l) < 0 || (l) > BITS_PER_LONG)) || \
(!__builtin_constant_p(h) && \
((h) < 0 || (h) > BITS_PER_LONG)) || \
(l) > (h), \
"GENMASK: invalid mask values: l: %u, h: %d\n", \
(l), (h)); \
if (low > high) \
swap(low, high); \
(((U32_C(1) << (high - low + 1)) - 1) << low); \
})

And maybe this should be renamed something like
#define BIT_MASK_RANGE(h, l)
or
#define BIT_MASK_SHIFTED(h, l)