On 09/10/19 05:14, Like Xu wrote: >> >> >>> I'm not sure is this your personal preference or is there a technical >>> reason such as this usage is not incompatible with union syntax? >> >> Apparently it 'works', so there is no hard technical reason, but >> consider that _Bool is specified as an integer type large enough to >> store the values 0 and 1, then consider it as a base type for a >> bitfield. That's just disguisting. > > It's reasonable. Thanks. /me chimes in since this is KVM code after all... For stuff like hardware registers, bitfields are probably a bad idea anyway, so let's only consider the case of space optimization. bool:2 would definitely cause an eyebrow raise, but I don't see why bool:1 bitfields are a problem. An integer type large enough to store the values 0 and 1 can be of any size bigger than one bit. bool bitfields preserve the magic behavior where something like this: foo->x = y; (x is a bool bitfield) would be compiled as foo->x = (y != 0); which can be a plus or a minus depending on the point of view. :) Either way, bool bitfields are useful if you are using bitfields for space optimization, especially if you have existing code using bool and it might rely on the idiom above. However, in this patch bitfields are unnecessary and they result in worse code from the compiler. There is plenty of padding in struct kvm_pmu, with or without bitfields, so I'd go with "u8 event_count; bool enable_cleanup;" (or better "need_cleanup"). Thanks, Paolo
