Re: [XEN PATCH 7/7] x86/xstate: move BUILD_BUG_ON to address MISRA C:2012 Rule 2.1

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 11 Dec 2023, Nicola Vetrini wrote:
> The string literal inside the expansion of BUILD_BUG_ON is considered
> unreachable code; however, such statement can be moved earlier
> with no functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> ---
> The motivation for this code movement is that keeping it inside the switch
> statement matches MISRA's definition of unreachable code, but does not fall 
> into
> the category of declarations without initialization, which is already a 
> deviated
> aspect. An alternative approach would be to deviate BUILD_BUG_ON as well.

I think that deviating BUILD_BUG_ON would be totally fine. But given
that this patch is obviously correct:

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>  xen/arch/x86/xstate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
> index cf94761d0542..99f0526c8988 100644
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -396,9 +396,10 @@ void xrstor(struct vcpu *v, uint64_t mask)
>       */
>      for ( prev_faults = faults = 0; ; prev_faults = faults )
>      {
> +        BUILD_BUG_ON(sizeof(faults) != 4); /* Clang doesn't support %z in 
> asm. */
> +
>          switch ( __builtin_expect(ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET], 8) )
>          {
> -            BUILD_BUG_ON(sizeof(faults) != 4); /* Clang doesn't support %z 
> in asm. */
>  #define _xrstor(insn) \
>          asm volatile ( "1: .byte " insn "\n" \
>                         "3:\n" \
> -- 
> 2.34.1
>