1. Patchew
  2. Xen
  3. View series

[XEN PATCH] docs/misra: add x86_64 and arm64 asm-offset.c to exclude-list

Nicola Vetrini posted 1 patch 2 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/b0c855581eed247a32b745906f84d352bf812091.1707324479.git.nicola.vetrini@bugseng.com
docs/misra/exclude-list.json | 8 ++++++++
1 file changed, 8 insertions(+)
[XEN PATCH] docs/misra: add x86_64 and arm64 asm-offset.c to exclude-list
Posted by Nicola Vetrini 2 months, 3 weeks ago 
These two files contain several deliberate violations of MISRA C rules and
they are not linked in the final Xen binary, therefore they can be exempted
from MISRA compliance.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Since the exclude list only contains arm64 and x86 files I reasoned that
introducing an entry that would match all architectures would not be desirable
(e.g., arm32). I'm happy to change that, though.
---
 docs/misra/exclude-list.json | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
index 7971d0e70f5b..bd05478e03e9 100644
--- a/docs/misra/exclude-list.json
+++ b/docs/misra/exclude-list.json
@@ -17,6 +17,10 @@
             "rel_path": "arch/arm/arm64/lib/find_next_bit.c",
             "comment": "Imported from Linux, ignore for now"
         },
+        {
+          "rel_path": "arch/arm/arm64/asm-offsets.c",
+          "comment": "The resulting code is not included in the final Xen binary, ignore for now"
+        },
         {
             "rel_path": "arch/x86/acpi/boot.c",
             "comment": "Imported from Linux, ignore for now"
@@ -97,6 +101,10 @@
             "rel_path": "arch/x86/x86_64/mmconf-fam10h.c",
             "comment": "Imported from Linux, ignore for now"
         },
+        {
+          "rel_path": "arch/x86/x86_64/asm-offsets.c",
+          "comment": "The resulting code is not included in the final Xen binary, ignore for now"
+        },
         {
             "rel_path": "arch/x86/efi/check.c",
             "comment": "The resulting code is not included in the final Xen binary, ignore for now"
-- 
2.34.1
Re: [XEN PATCH] docs/misra: add x86_64 and arm64 asm-offset.c to exclude-list
Posted by Julien Grall 2 months, 3 weeks ago 
Hi Nicola,

On 08/02/2024 07:55, Nicola Vetrini wrote:
> These two files contain several deliberate violations of MISRA C rules and
> they are not linked in the final Xen binary, therefore they can be exempted
> from MISRA compliance.

I am curious, what are the violations you are talking about?

Cheers,

>  > No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
> Since the exclude list only contains arm64 and x86 files I reasoned that
> introducing an entry that would match all architectures would not be desirable
> (e.g., arm32). I'm happy to change that, though.
> ---
>   docs/misra/exclude-list.json | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
> index 7971d0e70f5b..bd05478e03e9 100644
> --- a/docs/misra/exclude-list.json
> +++ b/docs/misra/exclude-list.json
> @@ -17,6 +17,10 @@
>               "rel_path": "arch/arm/arm64/lib/find_next_bit.c",
>               "comment": "Imported from Linux, ignore for now"
>           },
> +        {
> +          "rel_path": "arch/arm/arm64/asm-offsets.c",
> +          "comment": "The resulting code is not included in the final Xen binary, ignore for now"
> +        },
>           {
>               "rel_path": "arch/x86/acpi/boot.c",
>               "comment": "Imported from Linux, ignore for now"
> @@ -97,6 +101,10 @@
>               "rel_path": "arch/x86/x86_64/mmconf-fam10h.c",
>               "comment": "Imported from Linux, ignore for now"
>           },
> +        {
> +          "rel_path": "arch/x86/x86_64/asm-offsets.c",
> +          "comment": "The resulting code is not included in the final Xen binary, ignore for now"
> +        },
>           {
>               "rel_path": "arch/x86/efi/check.c",
>               "comment": "The resulting code is not included in the final Xen binary, ignore for now"

-- 
Julien Grall
Re: [XEN PATCH] docs/misra: add x86_64 and arm64 asm-offset.c to exclude-list
Posted by Nicola Vetrini 2 months, 3 weeks ago 
Hi Julien,

On 2024-02-08 22:33, Julien Grall wrote:
> Hi Nicola,
> 
> On 08/02/2024 07:55, Nicola Vetrini wrote:
>> These two files contain several deliberate violations of MISRA C rules 
>> and
>> they are not linked in the final Xen binary, therefore they can be 
>> exempted
>> from MISRA compliance.
> 
> I am curious, what are the violations you are talking about?
> 
> Cheers,
> 

The one that prompted the exclusion is for R20.12 on arm for the macros 
DEFINE and OFFSET, where the second argument of OFFSET is a macro and is 
used as a normal parameter and a stringification operand.
However, there are other special cases (e.g., the file not being linked, 
which violates R2.1 and was already decided to deviate that aspect).

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH] docs/misra: add x86_64 and arm64 asm-offset.c to exclude-list
Posted by Julien Grall 2 months, 3 weeks ago 

On 09/02/2024 07:52, Nicola Vetrini wrote:
> Hi Julien,

Hi Nicola,

> 
> On 2024-02-08 22:33, Julien Grall wrote:
>> Hi Nicola,
>>
>> On 08/02/2024 07:55, Nicola Vetrini wrote:
>>> These two files contain several deliberate violations of MISRA C 
>>> rules and
>>> they are not linked in the final Xen binary, therefore they can be 
>>> exempted
>>> from MISRA compliance.
>>
>> I am curious, what are the violations you are talking about?
>>
>> Cheers,
>>
> 
> The one that prompted the exclusion is for R20.12 on arm for the macros 
> DEFINE and OFFSET, where the second argument of OFFSET is a macro and is 
> used as a normal parameter and a stringification operand.
> However, there are other special cases (e.g., the file not being linked, 
> which violates R2.1 and was already decided to deviate that aspect).

Thanks for the clarification. I think it would be worth to add what you 
just wrote in the commit message. This give an idea to the reviewer why 
we excluded it.

With Jan's request addressed:

Acked-by: Julien Grall <jgrall@amazon.com>

> 

-- 
Julien Grall
Re: [XEN PATCH] docs/misra: add x86_64 and arm64 asm-offset.c to exclude-list
Posted by Nicola Vetrini 2 months, 3 weeks ago 
On 2024-02-09 10:28, Julien Grall wrote:
> On 09/02/2024 07:52, Nicola Vetrini wrote:
>> Hi Julien,
> 
> Hi Nicola,
> 
>> 
>> On 2024-02-08 22:33, Julien Grall wrote:
>>> Hi Nicola,
>>> 
>>> On 08/02/2024 07:55, Nicola Vetrini wrote:
>>>> These two files contain several deliberate violations of MISRA C 
>>>> rules and
>>>> they are not linked in the final Xen binary, therefore they can be 
>>>> exempted
>>>> from MISRA compliance.
>>> 
>>> I am curious, what are the violations you are talking about?
>>> 
>>> Cheers,
>>> 
>> 
>> The one that prompted the exclusion is for R20.12 on arm for the 
>> macros DEFINE and OFFSET, where the second argument of OFFSET is a 
>> macro and is used as a normal parameter and a stringification operand.
>> However, there are other special cases (e.g., the file not being 
>> linked, which violates R2.1 and was already decided to deviate that 
>> aspect).
> 
> Thanks for the clarification. I think it would be worth to add what you 
> just wrote in the commit message. This give an idea to the reviewer why 
> we excluded it.
> 
> With Jan's request addressed:
> 
> Acked-by: Julien Grall <jgrall@amazon.com>
> 
>> 

Thanks. I did send a v2 of this patch addressing Jan's request. I'll let 
him decide whether he wants a more detailed commit message.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH] docs/misra: add x86_64 and arm64 asm-offset.c to exclude-list
Posted by Julien Grall 2 months, 3 weeks ago 
Hi,

On 09/02/2024 09:34, Nicola Vetrini wrote:
> On 2024-02-09 10:28, Julien Grall wrote:
>> On 09/02/2024 07:52, Nicola Vetrini wrote:
>>> Hi Julien,
>>
>> Hi Nicola,
>>
>>>
>>> On 2024-02-08 22:33, Julien Grall wrote:
>>>> Hi Nicola,
>>>>
>>>> On 08/02/2024 07:55, Nicola Vetrini wrote:
>>>>> These two files contain several deliberate violations of MISRA C 
>>>>> rules and
>>>>> they are not linked in the final Xen binary, therefore they can be 
>>>>> exempted
>>>>> from MISRA compliance.
>>>>
>>>> I am curious, what are the violations you are talking about?
>>>>
>>>> Cheers,
>>>>
>>>
>>> The one that prompted the exclusion is for R20.12 on arm for the 
>>> macros DEFINE and OFFSET, where the second argument of OFFSET is a 
>>> macro and is used as a normal parameter and a stringification operand.
>>> However, there are other special cases (e.g., the file not being 
>>> linked, which violates R2.1 and was already decided to deviate that 
>>> aspect).
>>
>> Thanks for the clarification. I think it would be worth to add what 
>> you just wrote in the commit message. This give an idea to the 
>> reviewer why we excluded it.
>>
>> With Jan's request addressed:
>>
>> Acked-by: Julien Grall <jgrall@amazon.com>
>>
>>>
> 
> Thanks. I did send a v2 of this patch addressing Jan's request. I'll let 
> him decide whether he wants a more detailed commit message.

Ah I didn't spot the v2. Jan, would you be happy if I commit it with the 
updated commit message?

Cheers,

-- 
Julien Grall
Re: [XEN PATCH] docs/misra: add x86_64 and arm64 asm-offset.c to exclude-list
Posted by Jan Beulich 2 months, 3 weeks ago 
On 08.02.2024 08:55, Nicola Vetrini wrote:
> These two files contain several deliberate violations of MISRA C rules and
> they are not linked in the final Xen binary, therefore they can be exempted
> from MISRA compliance.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
> Since the exclude list only contains arm64 and x86 files I reasoned that
> introducing an entry that would match all architectures would not be desirable
> (e.g., arm32). I'm happy to change that, though.

Just wanted to ask - if globs are permitted, I'd favor covering all possible
architectures. It is certainly expected that they all follow suit. Just that
in the absence of sub-architectures the path would be xen/arch/*/asm-offsets.c
(and it's quite possible that we may, over time, morph x86 to a sub-arch-less
form).

Jan

> ---
>  docs/misra/exclude-list.json | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
> index 7971d0e70f5b..bd05478e03e9 100644
> --- a/docs/misra/exclude-list.json
> +++ b/docs/misra/exclude-list.json
> @@ -17,6 +17,10 @@
>              "rel_path": "arch/arm/arm64/lib/find_next_bit.c",
>              "comment": "Imported from Linux, ignore for now"
>          },
> +        {
> +          "rel_path": "arch/arm/arm64/asm-offsets.c",
> +          "comment": "The resulting code is not included in the final Xen binary, ignore for now"
> +        },
>          {
>              "rel_path": "arch/x86/acpi/boot.c",
>              "comment": "Imported from Linux, ignore for now"
> @@ -97,6 +101,10 @@
>              "rel_path": "arch/x86/x86_64/mmconf-fam10h.c",
>              "comment": "Imported from Linux, ignore for now"
>          },
> +        {
> +          "rel_path": "arch/x86/x86_64/asm-offsets.c",
> +          "comment": "The resulting code is not included in the final Xen binary, ignore for now"
> +        },
>          {
>              "rel_path": "arch/x86/efi/check.c",
>              "comment": "The resulting code is not included in the final Xen binary, ignore for now"
Re: [XEN PATCH] docs/misra: add x86_64 and arm64 asm-offset.c to exclude-list
Posted by Nicola Vetrini 2 months, 3 weeks ago 
Hi Jan,

On 2024-02-08 09:05, Jan Beulich wrote:
> On 08.02.2024 08:55, Nicola Vetrini wrote:
>> These two files contain several deliberate violations of MISRA C rules 
>> and
>> they are not linked in the final Xen binary, therefore they can be 
>> exempted
>> from MISRA compliance.
>> 
>> No functional change.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> ---
>> Since the exclude list only contains arm64 and x86 files I reasoned 
>> that
>> introducing an entry that would match all architectures would not be 
>> desirable
>> (e.g., arm32). I'm happy to change that, though.
> 
> Just wanted to ask - if globs are permitted, I'd favor covering all 
> possible
> architectures. It is certainly expected that they all follow suit. Just 
> that
> in the absence of sub-architectures the path would be 
> xen/arch/*/asm-offsets.c
> (and it's quite possible that we may, over time, morph x86 to a 
> sub-arch-less
> form).
> 
> Jan
> 

Ok, I'll modify it. Thanks,

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.