| Message ID | cover.1703066935.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | xen/arm: address violations of MISRA C:2012 Rule 16.3 | expand |
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
On 20.12.2023 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.: Did you mean "blank line" here, seeing ... > ... > break; > > case ...: > > This is to prevent larger switch statements from being a straight wall > of text. ... this as the further explanation? Jan
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.
On 21/12/2023 8:08 am, Jan Beulich wrote: > On 20.12.2023 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.: > Did you mean "blank line" here, seeing ... > >> ... >> break; >> >> case ...: >> >> This is to prevent larger switch statements from being a straight wall >> of text. > ... this as the further explanation? Urgh yes - I did mean blank line. Hopefully the intent was clear. ~Andrew
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(-)