Re: [XEN PATCH v2 4/9] x86/efi: tidy switch statement and address MISRA violation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.04.2024 11:14, Nicola Vetrini wrote:
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -169,20 +169,22 @@ static void __init 
> efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
>  
>          switch ( desc->Type )
>          {
> +        default:
> +            type = E820_RESERVED;
> +            break;

This one I guess is tolerable duplication-wise, and I might guess others would
even have preferred it like that from the beginning. A blank line below here
would be nice, though (and at some point ahead of and between the two ACPI-
related case labels blank lines would want adding, too).

However, ...

>          case EfiBootServicesCode:
>          case EfiBootServicesData:
>              if ( map_bs )
>              {
> -        default:
>                  type = E820_RESERVED;
>                  break;
>              }
> -            /* fall through */
> +            fallthrough;
>          case EfiConventionalMemory:
>              if ( !trampoline_phys && desc->PhysicalStart + len <= 0x100000 &&
>                   len >= cfg.size && desc->PhysicalStart + len > cfg.addr )
>                  cfg.addr = (desc->PhysicalStart + len - cfg.size) & 
> PAGE_MASK;
> -            /* fall through */
> +            fallthrough;
>          case EfiLoaderCode:
>          case EfiLoaderData:
>              if ( desc->Attribute & EFI_MEMORY_RUNTIME )

... below here there is

            else
        case EfiUnusableMemory:
                type = E820_UNUSABLE;
            break;

I understand there are no figure braces here, and hence be the letter this
isn't an issue with the Misra rule. But (a) some (e.g. Andrew, I guess)
would likely argue for there wanting to be braces and (b) we don't want to
be leaving this as is, when the spirit of the rule still suggests it should
be done differently.

Jan