diff mbox series

[XEN,for-4.19,v2,8/8] xen/compat: use BUILD_BUG_ON in CHECK_SIZE macros

Message ID 6138e02935236afd51a5db98d3527e5e2602468d.1697123806.git.nicola.vetrini@bugseng.com (mailing list archive)
State Superseded
Headers show
Series address violations of MISRA C:2012 Rule 10.1 | expand

Commit Message

Nicola Vetrini Oct. 12, 2023, 3:28 p.m. UTC
BUILD_BUG_ON is the preferred way to induce a build error
upon statically determined incorrect conditions.

This also fixes a MISRA C:2012 Rule 10.1 violation in the
previous formulation.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Changes in v2:
- replace the construct with a BUILD_BUG_ON.
---
 xen/include/xen/compat.h | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Comments

Jan Beulich Oct. 17, 2023, 6:09 a.m. UTC | #1
On 12.10.2023 17:28, Nicola Vetrini wrote:
> BUILD_BUG_ON is the preferred way to induce a build error
> upon statically determined incorrect conditions.
> 
> This also fixes a MISRA C:2012 Rule 10.1 violation in the
> previous formulation.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Hmm, looking back it's indeed not clear why I didn't use BUILD_BUG_ON() right
away. Perhaps just to avoid inline functions when things can be done without.
And/or because originally the macros were intended to be usable in function
bodies, not (just) at file scope. However, ...

> --- a/xen/include/xen/compat.h
> +++ b/xen/include/xen/compat.h
> @@ -151,12 +151,20 @@ CHECK_NAME_(k, n, T)(k xen_ ## n *x, \
>      return x == c; \
>  }
>  
> -#define CHECK_SIZE(name) \
> -    typedef int CHECK_NAME(name, S)[1 - (sizeof(xen_ ## name ## _t) != \
> -                                         sizeof(compat_ ## name ## _t)) * 2]
> +#define CHECK_SIZE(name)                                  \
> +static inline void __maybe_unused CHECK_SIZE_##name(void) \
> +{                                                         \
> +    typedef int CHECK_NAME(name, S)[1];                   \

... what's this and ...

> +    BUILD_BUG_ON(sizeof(xen_ ## name ## _t) !=            \
> +                 sizeof(compat_ ## name ## _t));          \
> +}
>  #define CHECK_SIZE_(k, n) \
> -    typedef int CHECK_NAME_(k, n, S)[1 - (sizeof(k xen_ ## n) != \
> -                                          sizeof(k compat_ ## n)) * 2]
> +static inline void __maybe_unused CHECK_SIZE_##k_##n(void) \
> +{                                                          \
> +    typedef int CHECK_NAME_(k, n, S)[1];                   \

... this needed for? The types aren't used anywhere afaict.

Jan

> +    BUILD_BUG_ON(sizeof(k xen_ ## n) !=                    \
> +                 sizeof(k compat_ ## n));                  \
> +}
>  
>  #define CHECK_FIELD_COMMON(name, t, f) \
>  static inline int __maybe_unused name(xen_ ## t ## _t *x, compat_ ## t ## _t *c) \
Nicola Vetrini Oct. 19, 2023, 2:35 p.m. UTC | #2
On 17/10/2023 08:09, Jan Beulich wrote:
> On 12.10.2023 17:28, Nicola Vetrini wrote:
>> BUILD_BUG_ON is the preferred way to induce a build error
>> upon statically determined incorrect conditions.
>> 
>> This also fixes a MISRA C:2012 Rule 10.1 violation in the
>> previous formulation.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> Hmm, looking back it's indeed not clear why I didn't use BUILD_BUG_ON() 
> right
> away. Perhaps just to avoid inline functions when things can be done 
> without.
> And/or because originally the macros were intended to be usable in 
> function
> bodies, not (just) at file scope. However, ...
> 
>> --- a/xen/include/xen/compat.h
>> +++ b/xen/include/xen/compat.h
>> @@ -151,12 +151,20 @@ CHECK_NAME_(k, n, T)(k xen_ ## n *x, \
>>      return x == c; \
>>  }
>> 
>> -#define CHECK_SIZE(name) \
>> -    typedef int CHECK_NAME(name, S)[1 - (sizeof(xen_ ## name ## _t) 
>> != \
>> -                                         sizeof(compat_ ## name ## 
>> _t)) * 2]
>> +#define CHECK_SIZE(name)                                  \
>> +static inline void __maybe_unused CHECK_SIZE_##name(void) \
>> +{                                                         \
>> +    typedef int CHECK_NAME(name, S)[1];                   \
> 
> ... what's this and ...
> 
>> +    BUILD_BUG_ON(sizeof(xen_ ## name ## _t) !=            \
>> +                 sizeof(compat_ ## name ## _t));          \
>> +}
>>  #define CHECK_SIZE_(k, n) \
>> -    typedef int CHECK_NAME_(k, n, S)[1 - (sizeof(k xen_ ## n) != \
>> -                                          sizeof(k compat_ ## n)) * 
>> 2]
>> +static inline void __maybe_unused CHECK_SIZE_##k_##n(void) \
>> +{                                                          \
>> +    typedef int CHECK_NAME_(k, n, S)[1];                   \
> 
> ... this needed for? The types aren't used anywhere afaict.
> 
> Jan
> 
>> +    BUILD_BUG_ON(sizeof(k xen_ ## n) !=                    \
>> +                 sizeof(k compat_ ## n));                  \
>> +}
>> 
>>  #define CHECK_FIELD_COMMON(name, t, f) \
>>  static inline int __maybe_unused name(xen_ ## t ## _t *x, compat_ ## 
>> t ## _t *c) \

You're probably right. I was wondering the same thing when replacing the 
code with
BUILD_BUG_ON.
diff mbox series

Patch

diff --git a/xen/include/xen/compat.h b/xen/include/xen/compat.h
index f2ce5bb3580a..4daa04183eac 100644
--- a/xen/include/xen/compat.h
+++ b/xen/include/xen/compat.h
@@ -151,12 +151,20 @@  CHECK_NAME_(k, n, T)(k xen_ ## n *x, \
     return x == c; \
 }
 
-#define CHECK_SIZE(name) \
-    typedef int CHECK_NAME(name, S)[1 - (sizeof(xen_ ## name ## _t) != \
-                                         sizeof(compat_ ## name ## _t)) * 2]
+#define CHECK_SIZE(name)                                  \
+static inline void __maybe_unused CHECK_SIZE_##name(void) \
+{                                                         \
+    typedef int CHECK_NAME(name, S)[1];                   \
+    BUILD_BUG_ON(sizeof(xen_ ## name ## _t) !=            \
+                 sizeof(compat_ ## name ## _t));          \
+}
 #define CHECK_SIZE_(k, n) \
-    typedef int CHECK_NAME_(k, n, S)[1 - (sizeof(k xen_ ## n) != \
-                                          sizeof(k compat_ ## n)) * 2]
+static inline void __maybe_unused CHECK_SIZE_##k_##n(void) \
+{                                                          \
+    typedef int CHECK_NAME_(k, n, S)[1];                   \
+    BUILD_BUG_ON(sizeof(k xen_ ## n) !=                    \
+                 sizeof(k compat_ ## n));                  \
+}
 
 #define CHECK_FIELD_COMMON(name, t, f) \
 static inline int __maybe_unused name(xen_ ## t ## _t *x, compat_ ## t ## _t *c) \