Messages in this thread | | | Date | Wed, 20 Aug 2008 16:07:22 +0300 | From | Boaz Harrosh <> | Subject | Re: [PATCH] debug: fix BUILD_BUG_ON() for non-constant expressions |
| |
adobriyan@gmail.com wrote: > On Wed, Aug 20, 2008 at 03:31:53PM +0300, Boaz Harrosh wrote: >> Ingo Molnar wrote: >>> * Boaz Harrosh <bharrosh@panasas.com> wrote: >>> >>>> If the user of virtio_has_feature() must pass a compile-time constant >>>> then it must be converted to a MACRO, and then the BUILD_BUG_ON will >>>> work. Or it should be changed to a BUG_ON() if fbit is a runtime >>>> variable. >> The use of __builtin_constant_p in inline functions is broken. This >> is because it will give different results depending on the -O level >> used. So I think that using it in the Kernel with inlines is plain >> broken. And should be discouraged. > > See how kmalloc() works. > Right:
static inline void *kmalloc(size_t size, gfp_t flags) { if (__builtin_constant_p(size)) { ... do some stuff else return __kmalloc_xxx(..) }
So if optimization is on we might gain some cycles but we must make sure we have the proper else clause.
In any way virtio_has_feature() is broken and no amount of optimization may ever fix it. Please look at it:
static inline bool virtio_has_feature(const struct virtio_device *vdev, unsigned int fbit) { /* Did you forget to fix assumptions on max features? */ if (__builtin_constant_p(fbit)) BUILD_BUG_ON(sizeof(fbit) >= 32);
the __builtin_constant_p(fbit) will never help. sizeof(fbit) is always sizeof(unsigned int) in any case. Only a macro can help here. Sorry
Boaz
| |