xen/arm: address violations of MISRA C:2012 Rule 16.3

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

Posted by Federico Serafini 2 years, 1 month ago

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

-- 
2.34.1

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

Posted by Andrew Cooper 2 years, 1 month ago

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

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

Posted by Federico Serafini 2 years, 1 month ago

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)

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

Posted by Jan Beulich 2 years, 1 month ago

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

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

Posted by Andrew Cooper 2 years, 1 month ago

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