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

From: Stefano Stabellini <sstabellini@kernel.org>
To: Nicola Vetrini <nicola.vetrini@bugseng.com>
Cc: xen-devel@lists.xenproject.org, consulting@bugseng.com,
	"Jan Beulich" <jbeulich@suse.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>
Subject: Re: [XEN PATCH 7/7] x86/xstate: move BUILD_BUG_ON to address MISRA C:2012 Rule 2.1
Date: Mon, 11 Dec 2023 17:46:16 -0800 (PST)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2312111745280.1703076@ubuntu-linux-20-04-desktop> (raw)
In-Reply-To: <a969550faea681c69730c0968264781f7739670d.1702283415.git.nicola.vetrini@bugseng.com>

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@bugseng.com>
> ---
> 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@kernel.org>

> ---
>  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
> 