No it does not have this problem. Have you tested it?
I have! It works fine. (Complains on non-const expressions)
The problem with the original BUILD_BUG_ON was the
(void)() cast and the interaction with the optimizer.
So it has not been used and it will not in the future.
In macros just use multiple statements separated by ";"
and surrounded by do{}while(0). To reach the same effect.
Note that BUG_ON is a statement and now BUILD_BUG_ON() is also.
for expressions use BUILD_BUG_ON_ZERO()
OK I have tested with your version of BUILD_BUG_ON{,_ZERO} and I have
two problems with it.
[1] Problem number one is with this sequence of code:
(a)
static inline bool __virtio_has_feature(const struct virtio_device *vdev,
unsigned int fbit)
{
virtio_check_driver_offered_feature(vdev, fbit);
return test_bit(fbit, vdev->features);
}
/* Did you forget to fix assumptions on max features? */
#define virtio_has_feature(vdev, fbit) \
((__builtin_constant_p(fbit) ? BUILD_BUG_ON_ZERO(fbit >= 32) : 0) + \
__virtio_has_feature(vdev, fbit))
...
(b)
static inline int virtio_config_buf(struct virtio_device *vdev,
unsigned int fbit,
unsigned int offset,
void *buf, unsigned len)
{
if (!virtio_has_feature(vdev, fbit))
return -ENOENT;
...
(c)
if (data && !virtio_has_feature(vdev, VIRTIO_NET_F_CSUM))
return -ENOSYS;
I have not fount a way to code (a) in a way that actually works
for both (b) and (c). That is, good with const expressions and
bypassed with non-const. (See explanations below)
with your code:
The MAYBE_BUILD_BUG_ON just gets ignored in all cases and
coding (c) as:
if (data && !virtio_has_feature(vdev, 717))
return -ENOSYS;
Will not complain at all.
[2] Problem number two is with this sequence of code:
#define is_power_of_two(x) ( !((x) & ((x)-1)) )
#define low_bit_mask(x) ( ((x)-1) & ~(x) )
#define is_valid_mask(x) is_power_of_two(1 + (x) + \
low_bit_mask(x))
#define FIELD_CHECK(__mask, __type) \
BUILD_BUG_ON(!__builtin_constant_p(__mask) || \
!(__mask) || \
!is_valid_mask(__mask) || \
(__mask) != (__type)(__mask)) \
#define FIELD8(__mask) \
({ \
FIELD_CHECK(__mask, u8); \
(struct rt2x00_field8) { \
compile_ffs8(__mask), (__mask) \
}; \
})
I expect the compiler to complain since I'm abusing the
"sign-bit" and need unsigned calculations, but the
(void)() cast musks the all operation off. Actually the
all FIELD_CHECK is ignored, I have tried illegal values
and they are let through.
From what I understand the (void)() cast gets thrown away
by the optimizer very fast before it is fully generated hence
the silence in lots of "bad" cases. The BUILD_BUG_ON_ZERO does
not have this problem because it cannot be optimized out.
On the other hand with your bit field approach the constant-expression
is requested by the compiler very early and cannot be
"optimized out" earlier like in the problem [1] case.
I agree that this is all very ugly in theory. But in practice
it is simple, works(*), and lets me do things like problem[1]
that still produce results.
(*) By works I mean:
Has no false-positives and no false-negatives. On two accounts:
constness is enforced, condition is fully checked.
Please show example code that fails my bold (maybe even blunt)
statements, where you would expect one result and the compiler
does something else.
Boaz
--
