On 11/10/2023 12:06 am, Nicola Vetrini wrote: > On 10/10/2023 18:00, Andrew Cooper wrote: >> On 10/10/2023 9:02 am, Stefano Stabellini wrote: >>> On Fri, 6 Oct 2023, Nicola Vetrini wrote: >>>> The essential type of the result of an inequality operator is >>>> essentially boolean, therefore it shouldn't be used as an argument of >>>> the multiplication operator, which expects an integer. >>>> >>>> Signed-off-by: Nicola Vetrini <nicola.vetr...@bugseng.com> >>>> --- >>>> xen/include/xen/compat.h | 10 ++++++---- >>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/xen/include/xen/compat.h b/xen/include/xen/compat.h >>>> index f2ce5bb3580a..5ffee6a9fed1 100644 >>>> --- a/xen/include/xen/compat.h >>>> +++ b/xen/include/xen/compat.h >>>> @@ -151,12 +151,14 @@ CHECK_NAME_(k, n, T)(k xen_ ## n *x, \ >>>> return x == c; \ >>>> } >>>> >>>> +#define SIZE_NEQUAL(a, b) \ >>>> + (sizeof(a) != sizeof(b) ? 1 : 0) >>>> #define CHECK_SIZE(name) \ >>>> - typedef int CHECK_NAME(name, S)[1 - (sizeof(xen_ ## name ## >>>> _t) != \ >>>> - sizeof(compat_ ## name ## >>>> _t)) * 2] >>>> + typedef int CHECK_NAME(name, S)[1 - (SIZE_NEQUAL(xen_ ## name >>>> ## _t, \ >>>> + compat_ ## >>>> name ## _t)) * 2] >>>> #define CHECK_SIZE_(k, n) \ >>>> - typedef int CHECK_NAME_(k, n, S)[1 - (sizeof(k xen_ ## n) != \ >>>> - sizeof(k compat_ ## n)) >>>> * 2] >>>> + typedef int CHECK_NAME_(k, n, S)[1 - (SIZE_NEQUAL(k xen_ ## n, \ >>>> + k compat_ ## >>>> n)) * 2] >>> I think this style is easier to read but I'll let the x86 maintainers >>> decide >>> >>> typedef int CHECK_NAME(name, S)[(sizeof(xen_ ## name ## _t) == \ >>> sizeof(compat_ ## name ## _t)) >>> ? 1 : -1] >>> >>> Also am I reading this correctly that we are using -1 as array index? I >>> must have made a calculation mistake? >> >> This is a NIH BUILD_BUG_ON(). It should be rewritten as >> >> BUILD_BUG_ON(sizeof(xen ...) != sizeof(compat ...)); >> >> which will use _Static_assert() in modern compilers. >> >> ~Andrew > > Ok, thanks. >
I'm going to go out on a limb and say that every other pattern that looks like this probably wants converting to a BUILD_BUG_ON(). This code quite possibly predates the introduction of BUILD_BUG_ON(), but we want to end up using BUILD_BUG_ON() everywhere because it's the construct that uses _Static_assert() wherever possible. ~Andrew
