Re: [XEN PATCH 0/7] xen/arm: address violations of MISRA C:2012 Rule 16.3

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 20/12/23 22:35, Andrew Cooper wrote:
> On 20/12/2023 11:03 am, Federico Serafini wrote:
> > This patch series addresses violations of MISRA C:2012 Rule 16.3 on the Arm
> > code. No fucntional changes are introduced.
> >
> > Federico Serafini (7):
> >    xen/arm: gic-v3: address violations of MISRA C:2012 Rule 16.3
> >    xen/arm: traps: address violations of MISRA C:2012 Rule 16.3
> >    xen/arm: guest_walk: address violations of MISRA C:2012 Rule 16.3
> >    xen/arm: mem_access: address violations of MISRA C:2012 Rule 16.3
> >    xen/arm: v{cp,sys}reg: address violations of MISRA C:2012 Rule 16.3
> >    xen/arm: mmu: address a violations of MISRA C:2012 Rule 16.3
> >    xen/arm: smmu-v3: address violations of MISRA C:2012 Rule 16.3
> >
> >   xen/arch/arm/arm64/vsysreg.c          |  4 ++--
> >   xen/arch/arm/gic-v3.c                 | 30 +++++++++++++++++++++++++++
> >   xen/arch/arm/guest_walk.c             |  4 ++++
> >   xen/arch/arm/mem_access.c             | 12 +++++------
> >   xen/arch/arm/mmu/p2m.c                |  1 +
> >   xen/arch/arm/traps.c                  | 18 ++++++++++++----
> >   xen/arch/arm/vcpreg.c                 |  4 ++--
> >   xen/drivers/passthrough/arm/smmu-v3.c |  2 ++
> >   8 files changed, 61 insertions(+), 14 deletions(-)
> Just a couple of notes on style.  This isn't a request to change
> anything in this series, particularly as most is already committed, but
> bear it in mind for what I expect will be similar patches in other areas.
>
> We explicitly permit tabulation when it aids readibility, so patch 2
> could have been written:
>
>          switch ( hypercall_args[*nr] ) {
>          case 5: HYPERCALL_ARG5(regs) = 0xDEADBEEFU; fallthrough;
>          case 4: HYPERCALL_ARG4(regs) = 0xDEADBEEFU; fallthrough;
>          case 3: HYPERCALL_ARG3(regs) = 0xDEADBEEFU; fallthrough;
>          case 2: HYPERCALL_ARG2(regs) = 0xDEADBEEFU; fallthrough;
>          case 1: /* Don't clobber x0/r0 -- it's the return value */
>          case 0: /* -ENOSYS case */
>              break;
>          default: BUG();
>          }
>
> (give or take the brace placement other style issue)  We also have cases
> where a break before a new case statement is preferred, i.e.:
>
>          ...
>          break;
>
>      case ...:
>
> This is to prevent larger switch statements from being a straight wall
> of text.
>
> If in doubt, match the style around it.  Please don't de-tabulate
> examples which are already tabulated.  (i.e. don't de-tabulate the x86
> versions of patch 2.)
>
> ~Andrew
Understood, thank you.

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)