Re: Question on FIELD_PREP() for static array
From: Johannes Berg <johannes@sipsolutions.net>
Date: 2018-10-10 18:14:00
Also in:
lkml
Subsystem:
bitmap api, the rest · Maintainers:
Yury Norov, Linus Torvalds
On Wed, 2018-10-10 at 10:33 -0700, Joe Perches wrote:
quoted
Specifically it doesn't like the __BF_FIELD_CHECK() in FIELD_PREP(). Any ideas on compiler trickery we could do with the FIELD_PREP() definition to avoid this issue (i.e. enforce the check but only use the constant value)?Perhaps __bf_shf should not use __builtin_ffsll.
__bf_shf() is a constant expression, and is fine in this context.
The problem is the use of the compound statement here:
static int x[2] = {
({ (void)(0); 1; }),
0,
}
similarly fails to compile.
I've recently run into a similar situation, namely in
include/net/netlink.h, and the applicable way to solve it here would be
something like this:
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 3f1ef4450a7c..0680d641923f 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h@@ -49,19 +49,16 @@ #define __bf_shf(x) (__builtin_ffsll(x) - 1) +#define BUILD_BUG_ON_RET_ZERO(cond) (sizeof(char[1 - 2*!!(cond)]) - 1) +#define BUILD_BUG_ON_NOT_POW2_RET_ZERO(n) BUILD_BUG_ON_RET_ZERO(((n) & ((n) - 1)) != 0) + #define __BF_FIELD_CHECK(_mask, _reg, _val, _pfx) \ - ({ \ - BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), \ - _pfx "mask is not constant"); \ - BUILD_BUG_ON_MSG((_mask) == 0, _pfx "mask is zero"); \ - BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \ - ~((_mask) >> __bf_shf(_mask)) & (_val) : 0, \ - _pfx "value too large for the field"); \ - BUILD_BUG_ON_MSG((_mask) > (typeof(_reg))~0ull, \ - _pfx "type of reg too small for mask"); \ - __BUILD_BUG_ON_NOT_POWER_OF_2((_mask) + \ - (1ULL << __bf_shf(_mask))); \ - }) + BUILD_BUG_ON_RET_ZERO(!__builtin_constant_p(_mask)) + \ + BUILD_BUG_ON_RET_ZERO((_mask) == 0) + \ + BUILD_BUG_ON_RET_ZERO(__builtin_constant_p(_val) ? \ + ~((_mask) >> __bf_shf(_mask)) & (_val) : 0) + \ + BUILD_BUG_ON_RET_ZERO((_mask) > (typeof(_reg))~0ull) + \ + BUILD_BUG_ON_NOT_POW2_RET_ZERO((_mask) + (1ULL << __bf_shf(_mask))) /** * FIELD_FIT() - check if value fits in the field
@@ -85,10 +82,8 @@ * be combined with other fields of the bitfield using logical OR. */ #define FIELD_PREP(_mask, _val) \ - ({ \ - __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \ - ((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask); \ - }) + (__BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ") + \ + (((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask))) /** * FIELD_GET() - extract a bitfield element
Note that this is an incomplete patch - everything but FIELD_PREP will not compile with this. Also, BUILD_BUG_ON_RET_ZERO and BUILD_BUG_ON_NOT_POW2_RET_ZERO should probably have better names, or perhaps do the positive way that I did in __NLA_ENSURE, e.g. CONST_ASSERT()/CONST_ASSERT_IS_POWER_OF_2()? I guess they should go to build_bug.h as well... johannes