[XEN PATCH v2 1/3] xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1

Nicola Vetrini posted 3 patches 2 years, 7 months ago
There is a newer version of this series
[XEN PATCH v2 1/3] xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1
Posted by Nicola Vetrini 2 years, 7 months ago
In the files `xen/arch/arm/include/asm/arm(32|64)/flushtlb.h' there are a
few occurrences of nested '//' character sequences inside C-style comment
blocks, which violate Rule 3.1. The patch aims to resolve those by removing
the nested comments.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com
---
Changes:
- Resending the patch with the right maintainers in CC.
Changes in V2:
- Split the patch into a series and reworked the fix.
- Apply the fix to the arm32 `flushtlb.h' file, for consistency
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/arm/include/asm/arm32/flushtlb.h | 8 ++++----
 xen/arch/arm/include/asm/arm64/flushtlb.h | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/include/asm/arm32/flushtlb.h b/xen/arch/arm/include/asm/arm32/flushtlb.h
index 22ee3b317b..bcbeac590b 100644
--- a/xen/arch/arm/include/asm/arm32/flushtlb.h
+++ b/xen/arch/arm/include/asm/arm32/flushtlb.h
@@ -4,10 +4,10 @@
 /*
  * Every invalidation operation use the following patterns:
  *
- * DSB ISHST        // Ensure prior page-tables updates have completed
- * TLBI...          // Invalidate the TLB
- * DSB ISH          // Ensure the TLB invalidation has completed
- * ISB              // See explanation below
+ * DSB ISHST        Ensure prior page-tables updates have completed
+ * TLBI...          Invalidate the TLB
+ * DSB ISH          Ensure the TLB invalidation has completed
+ * ISB              See explanation below
  *
  * For Xen page-tables the ISB will discard any instructions fetched
  * from the old mappings.
diff --git a/xen/arch/arm/include/asm/arm64/flushtlb.h b/xen/arch/arm/include/asm/arm64/flushtlb.h
index 56c6fc763b..6066a2d703 100644
--- a/xen/arch/arm/include/asm/arm64/flushtlb.h
+++ b/xen/arch/arm/include/asm/arm64/flushtlb.h
@@ -4,10 +4,10 @@
 /*
  * Every invalidation operation use the following patterns:
  *
- * DSB ISHST        // Ensure prior page-tables updates have completed
- * TLBI...          // Invalidate the TLB
- * DSB ISH          // Ensure the TLB invalidation has completed
- * ISB              // See explanation below
+ * DSB ISHST        Ensure prior page-tables updates have completed
+ * TLBI...          Invalidate the TLB
+ * DSB ISH          Ensure the TLB invalidation has completed
+ * ISB              See explanation below
  *
  * ARM64_WORKAROUND_REPEAT_TLBI:
  * Modification of the translation table for a virtual address might lead to
-- 
2.34.1
Re: [XEN PATCH v2 1/3] xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1
Posted by Julien Grall 2 years, 7 months ago
Hi,

On 19/06/2023 10:56, Nicola Vetrini wrote:
> In the files `xen/arch/arm/include/asm/arm(32|64)/flushtlb.h' there are a
> few occurrences of nested '//' character sequences inside C-style comment
> blocks, which violate Rule 3.1. The patch aims to resolve those by removing
> the nested comments.

As I wrote in 
https://lore.kernel.org/xen-devel/f3fc1848-68ca-37a1-add2-e100b4773190@xen.org/, 
I am against replacing '//' with nothing. I have proposed to use ';' 
because this is also a valid way to comment in assembly.

> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com

There is a missing '>' at the end of the list.

> ---
> Changes:
> - Resending the patch with the right maintainers in CC.
> Changes in V2:
> - Split the patch into a series and reworked the fix.
> - Apply the fix to the arm32 `flushtlb.h' file, for consistency
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
>   xen/arch/arm/include/asm/arm32/flushtlb.h | 8 ++++----
>   xen/arch/arm/include/asm/arm64/flushtlb.h | 8 ++++----
>   2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/arm32/flushtlb.h b/xen/arch/arm/include/asm/arm32/flushtlb.h
> index 22ee3b317b..bcbeac590b 100644
> --- a/xen/arch/arm/include/asm/arm32/flushtlb.h
> +++ b/xen/arch/arm/include/asm/arm32/flushtlb.h
> @@ -4,10 +4,10 @@
>   /*
>    * Every invalidation operation use the following patterns:
>    *
> - * DSB ISHST        // Ensure prior page-tables updates have completed
> - * TLBI...          // Invalidate the TLB
> - * DSB ISH          // Ensure the TLB invalidation has completed
> - * ISB              // See explanation below
> + * DSB ISHST        Ensure prior page-tables updates have completed
> + * TLBI...          Invalidate the TLB
> + * DSB ISH          Ensure the TLB invalidation has completed
> + * ISB              See explanation below
>    *
>    * For Xen page-tables the ISB will discard any instructions fetched
>    * from the old mappings.
> diff --git a/xen/arch/arm/include/asm/arm64/flushtlb.h b/xen/arch/arm/include/asm/arm64/flushtlb.h
> index 56c6fc763b..6066a2d703 100644
> --- a/xen/arch/arm/include/asm/arm64/flushtlb.h
> +++ b/xen/arch/arm/include/asm/arm64/flushtlb.h
> @@ -4,10 +4,10 @@
>   /*
>    * Every invalidation operation use the following patterns:
>    *
> - * DSB ISHST        // Ensure prior page-tables updates have completed
> - * TLBI...          // Invalidate the TLB
> - * DSB ISH          // Ensure the TLB invalidation has completed
> - * ISB              // See explanation below
> + * DSB ISHST        Ensure prior page-tables updates have completed
> + * TLBI...          Invalidate the TLB
> + * DSB ISH          Ensure the TLB invalidation has completed
> + * ISB              See explanation below
>    *
>    * ARM64_WORKAROUND_REPEAT_TLBI:
>    * Modification of the translation table for a virtual address might lead to

Cheers,

-- 
Julien Grall
Re: [XEN PATCH v2 1/3] xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1
Posted by Jan Beulich 2 years, 7 months ago
On 19.06.2023 12:01, Julien Grall wrote:
> On 19/06/2023 10:56, Nicola Vetrini wrote:
>> In the files `xen/arch/arm/include/asm/arm(32|64)/flushtlb.h' there are a
>> few occurrences of nested '//' character sequences inside C-style comment
>> blocks, which violate Rule 3.1. The patch aims to resolve those by removing
>> the nested comments.
> 
> As I wrote in 
> https://lore.kernel.org/xen-devel/f3fc1848-68ca-37a1-add2-e100b4773190@xen.org/, 
> I am against replacing '//' with nothing. I have proposed to use ';' 
> because this is also a valid way to comment in assembly.

Are you sure about this? For gas most targets use ; as a statement separator,
not as a comment character. Afaics arm-* and aarch64-* are no exception there.

Jan
Re: [XEN PATCH v2 1/3] xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1
Posted by Julien Grall 2 years, 7 months ago
Hi,

On 19/06/2023 11:25, Jan Beulich wrote:
> On 19.06.2023 12:01, Julien Grall wrote:
>> On 19/06/2023 10:56, Nicola Vetrini wrote:
>>> In the files `xen/arch/arm/include/asm/arm(32|64)/flushtlb.h' there are a
>>> few occurrences of nested '//' character sequences inside C-style comment
>>> blocks, which violate Rule 3.1. The patch aims to resolve those by removing
>>> the nested comments.
>>
>> As I wrote in
>> https://lore.kernel.org/xen-devel/f3fc1848-68ca-37a1-add2-e100b4773190@xen.org/,
>> I am against replacing '//' with nothing. I have proposed to use ';'
>> because this is also a valid way to comment in assembly.
> 
> Are you sure about this? For gas most targets use ; as a statement separator,
> not as a comment character. Afaics arm-* and aarch64-* are no exception there.

GAS will not accept it. But the Arm compiler will [1]. This is good 
enough for me because I want to have a separator between the instruction 
and the comment.

Cheers,

[1] 
https://developer.arm.com/documentation/dui0473/m/structure-of-assembly-language-modules/syntax-of-source-lines-in-assembly-language

-- 
Julien Grall
Re: [XEN PATCH v2 1/3] xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1
Posted by Nicola Vetrini 2 years, 7 months ago
Hi,

On 19/06/23 12:29, Julien Grall wrote:
> Hi,
> 
> On 19/06/2023 11:25, Jan Beulich wrote:
>> On 19.06.2023 12:01, Julien Grall wrote:
>>> On 19/06/2023 10:56, Nicola Vetrini wrote:
>>>> In the files `xen/arch/arm/include/asm/arm(32|64)/flushtlb.h' there 
>>>> are a
>>>> few occurrences of nested '//' character sequences inside C-style 
>>>> comment
>>>> blocks, which violate Rule 3.1. The patch aims to resolve those by 
>>>> removing
>>>> the nested comments.
>>>
>>> As I wrote in
>>> https://lore.kernel.org/xen-devel/f3fc1848-68ca-37a1-add2-e100b4773190@xen.org/,
>>> I am against replacing '//' with nothing. I have proposed to use ';'
>>> because this is also a valid way to comment in assembly.
>>
>> Are you sure about this? For gas most targets use ; as a statement 
>> separator,
>> not as a comment character. Afaics arm-* and aarch64-* are no 
>> exception there.
> 
> GAS will not accept it. But the Arm compiler will [1]. This is good 
> enough for me because I want to have a separator between the instruction 
> and the comment.
> 
> Cheers,
> 
> [1] 
> https://developer.arm.com/documentation/dui0473/m/structure-of-assembly-language-modules/syntax-of-source-lines-in-assembly-language
> 

If no one has any objection about this I'll add the ';' comment delimiter.

Regards,

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)