Re: [XEN PATCH 02/11] x86: move declarations to address MISRA C:2012 Rule 2.1

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 2 Aug 2023, Nicola Vetrini wrote:
> Variable declarations between a switch statement guard and before
> any case label are unreachable code, and hence violate Rule 2.1:
> "A project shall not contain unreachable code".
> 
> Therefore the declarations are moved in the smallest enclosing
> scope, near other variable definitions.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> ---
>  xen/arch/x86/cpuid.c  |  3 +--
>  xen/arch/x86/domain.c | 23 +++++++++++------------
>  xen/arch/x86/irq.c    |  3 +--
>  xen/arch/x86/msr.c    |  3 +--
>  4 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
> index 455a09b2dd..90e1370e90 100644
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -37,6 +37,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>  {
>      const struct domain *d = v->domain;
>      const struct cpu_policy *p = d->arch.cpu_policy;
> +    const struct cpu_user_regs *regs;
>  
>      *res = EMPTY_LEAF;
>  
> @@ -136,8 +137,6 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>       */
>      switch ( leaf )
>      {
> -        const struct cpu_user_regs *regs;
> -
>      case 0x1:
>          /* TODO: Rework topology logic. */
>          res->b &= 0x00ffffffu;
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 5f66c2ae33..015f7b14ab 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -2268,6 +2268,17 @@ int domain_relinquish_resources(struct domain *d)
>  {
>      int ret;
>      struct vcpu *v;
> +    enum {
> +        PROG_iommu_pagetables = 1,
> +        PROG_shared,
> +        PROG_paging,
> +        PROG_vcpu_pagetables,
> +        PROG_xen,
> +        PROG_l4,
> +        PROG_l3,
> +        PROG_l2,
> +        PROG_done,
> +    };
>  
>      BUG_ON(!cpumask_empty(d->dirty_cpumask));
>  
> @@ -2291,18 +2302,6 @@ int domain_relinquish_resources(struct domain *d)
>  #define PROGRESS(x)                                                     \
>          d->arch.rel_priv = PROG_ ## x; /* Fallthrough */ case PROG_ ## x
>  
> -        enum {
> -            PROG_iommu_pagetables = 1,
> -            PROG_shared,
> -            PROG_paging,
> -            PROG_vcpu_pagetables,
> -            PROG_xen,
> -            PROG_l4,
> -            PROG_l3,
> -            PROG_l2,
> -            PROG_done,
> -        };
> -
>      case 0:
>          ret = pci_release_devices(d);
>          if ( ret )
> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> index 6abfd81621..4fd0cc163d 100644
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -1135,6 +1135,7 @@ static void cf_check irq_guest_eoi_timer_fn(void *data)
>      struct irq_desc *desc = data;
>      unsigned int i, irq = desc - irq_desc;
>      irq_guest_action_t *action;
> +    cpumask_t *cpu_eoi_map;
>  
>      spin_lock_irq(&desc->lock);
>      
> @@ -1169,8 +1170,6 @@ static void cf_check irq_guest_eoi_timer_fn(void *data)
>  
>      switch ( action->ack_type )
>      {
> -        cpumask_t *cpu_eoi_map;

It is only used by case ACKTYPE_EOI so it can be moved there (with a new
block):


    case ACKTYPE_EOI:
    {
        cpumask_t *cpu_eoi_map = this_cpu(scratch_cpumask);
        cpumask_copy(cpu_eoi_map, action->cpu_eoi_map);
        spin_unlock_irq(&desc->lock);
        on_selected_cpus(cpu_eoi_map, set_eoi_ready, desc, 0);
        return;
    }
    }


>      case ACKTYPE_UNMASK:
>          if ( desc->handler->end )
>              desc->handler->end(desc, 0);
> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
> index ecf126566d..0e61e0fe4e 100644
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -335,11 +335,10 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t 
> val)
>      const struct cpu_policy *cp = d->arch.cpu_policy;
>      struct vcpu_msrs *msrs = v->arch.msrs;
>      int ret = X86EMUL_OKAY;
> +    uint64_t rsvd;
>  
>      switch ( msr )
>      {
> -        uint64_t rsvd;
> -

It is only used by case MSR_INTEL_MISC_FEATURES_ENABLES so it can be
moved there


>          /* Read-only */
>      case MSR_IA32_PLATFORM_ID:
>      case MSR_CORE_CAPABILITIES:
> -- 
> 2.34.1
>