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

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

[Nicola Vetrini]

These 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>
---
Changes in v2:
- use a glob to exclude this file for all architectures.
---
 docs/misra/exclude-list.json | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
index 7971d0e70f5b..89df966eeac9 100644
--- a/docs/misra/exclude-list.json
+++ b/docs/misra/exclude-list.json
@@ -101,6 +101,10 @@
             "rel_path": "arch/x86/efi/check.c",
             "comment": "The resulting code is not included in the final Xen binary, ignore for now"
         },
+        {
+          "rel_path": "arch/*/*/asm-offsets.c",
+          "comment": "The resulting code is not included in the final Xen binary, ignore for now"
+        },
         {
             "rel_path": "common/coverage/*",
             "comment": "Files to support gcov, ignore for now"
-- 
2.34.1

Re: [XEN PATCH v2] docs/misra: add asm-offset.c to exclude-list

[Jan Beulich]

On 08.02.2024 16:50, Nicola Vetrini wrote:
> These 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>

Acked-by: Jan Beulich <jbeulich@suse.com>
albeit ...

> --- a/docs/misra/exclude-list.json
> +++ b/docs/misra/exclude-list.json
> @@ -101,6 +101,10 @@
>              "rel_path": "arch/x86/efi/check.c",
>              "comment": "The resulting code is not included in the final Xen binary, ignore for now"
>          },
> +        {
> +          "rel_path": "arch/*/*/asm-offsets.c",
> +          "comment": "The resulting code is not included in the final Xen binary, ignore for now"
> +        },
>          {
>              "rel_path": "common/coverage/*",
>              "comment": "Files to support gcov, ignore for now"

... something looks odd with indentation; can probably be adjusted
while committing.

Jan

Re: [XEN PATCH v2] docs/misra: add asm-offset.c to exclude-list

[Nicola Vetrini]

On 2024-02-08 16:56, Jan Beulich wrote:
> On 08.02.2024 16:50, Nicola Vetrini wrote:
>> These 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>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> albeit ...
> 
>> --- a/docs/misra/exclude-list.json
>> +++ b/docs/misra/exclude-list.json
>> @@ -101,6 +101,10 @@
>>              "rel_path": "arch/x86/efi/check.c",
>>              "comment": "The resulting code is not included in the 
>> final Xen binary, ignore for now"
>>          },
>> +        {
>> +          "rel_path": "arch/*/*/asm-offsets.c",
>> +          "comment": "The resulting code is not included in the final 
>> Xen binary, ignore for now"
>> +        },
>>          {
>>              "rel_path": "common/coverage/*",
>>              "comment": "Files to support gcov, ignore for now"
> 
> ... something looks odd with indentation; can probably be adjusted
> while committing.
> 
> Jan

Sorry, I didn't notice the wrong indentation.

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

Re: [XEN PATCH v2] docs/misra: add asm-offset.c to exclude-list

[Julien Grall]

Hi,

Replying on the v2 as well.

On 08/02/2024 15:56, Jan Beulich wrote:
> On 08.02.2024 16:50, Nicola Vetrini wrote:
>> These 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'd like the commit message to be expanded a little bit to explain which 
MISRA rules are a problem. This helped me to understand why we excluded 
rather than fixed.

Base on the previous discussion, I would suggest:

These files contain several deliberate violation of MISRA C rules such as:
   * R20.12 on Arm for macros DEFINE and OFFSET, where the second 
argument of OFFSET is a macro and is used as a normal parameter and a 
stringification operand.
   * R2.1 because the file is not linked That said it was decided to 
deviate the rule itselfed to deviate that aspect).

The files are also not linked in the final Xen binary, therefore they 
can be expempted from MISRA compliance.

>>
>> No functional change.
>>
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> albeit ...
> 
>> --- a/docs/misra/exclude-list.json
>> +++ b/docs/misra/exclude-list.json
>> @@ -101,6 +101,10 @@
>>               "rel_path": "arch/x86/efi/check.c",
>>               "comment": "The resulting code is not included in the final Xen binary, ignore for now"
>>           },
>> +        {
>> +          "rel_path": "arch/*/*/asm-offsets.c",
>> +          "comment": "The resulting code is not included in the final Xen binary, ignore for now"
>> +        },
>>           {
>>               "rel_path": "common/coverage/*",
>>               "comment": "Files to support gcov, ignore for now"
> 
> ... something looks odd with indentation; can probably be adjusted
> while committing.

I am happy to take care of both the commit message and the indentation 
on commit.

Cheers,

-- 
Julien Grall

Re: [XEN PATCH v2] docs/misra: add asm-offset.c to exclude-list

[Jan Beulich]

On 09.02.2024 10:40, Julien Grall wrote:
> Replying on the v2 as well.

And answering here despite the respective question was raised on the
v1 thread: I'm certainly okay with the more detailed commit message.
A few nits, though:

> On 08/02/2024 15:56, Jan Beulich wrote:
>> On 08.02.2024 16:50, Nicola Vetrini wrote:
>>> These 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'd like the commit message to be expanded a little bit to explain which 
> MISRA rules are a problem. This helped me to understand why we excluded 
> rather than fixed.
> 
> Base on the previous discussion, I would suggest:
> 
> These files contain several deliberate violation of MISRA C rules such as:

violations

>    * R20.12 on Arm for macros DEFINE and OFFSET, where the second 
> argument of OFFSET is a macro and is used as a normal parameter and a 
> stringification operand.

Is this really for Arm only?

>    * R2.1 because the file is not linked That said it was decided to 
> deviate the rule itselfed to deviate that aspect).

There look to be punctuation issues here. Also s/itselfed/itself/, and
the duplicate "deviate" is also a little odd to read (maybe "deal with"
or "address" in place of the 2nd instance).

> The files are also not linked in the final Xen binary, therefore they 
> can be expempted from MISRA compliance.

Looks to duplicate what the latter half of the 2nd bullet point has.
If to be kept: s/expempted/exempted/.

>>> --- a/docs/misra/exclude-list.json
>>> +++ b/docs/misra/exclude-list.json
>>> @@ -101,6 +101,10 @@
>>>               "rel_path": "arch/x86/efi/check.c",
>>>               "comment": "The resulting code is not included in the final Xen binary, ignore for now"
>>>           },
>>> +        {
>>> +          "rel_path": "arch/*/*/asm-offsets.c",
>>> +          "comment": "The resulting code is not included in the final Xen binary, ignore for now"
>>> +        },
>>>           {
>>>               "rel_path": "common/coverage/*",
>>>               "comment": "Files to support gcov, ignore for now"
>>
>> ... something looks odd with indentation; can probably be adjusted
>> while committing.
> 
> I am happy to take care of both the commit message and the indentation 
> on commit.

Okay, I'll leave that to you then.

Jan

Re: [XEN PATCH v2] docs/misra: add asm-offset.c to exclude-list

[Julien Grall]

Hi Jan,

On 09/02/2024 11:46, Jan Beulich wrote:
> On 09.02.2024 10:40, Julien Grall wrote:
>> Replying on the v2 as well.
> 
> And answering here despite the respective question was raised on the
> v1 thread: I'm certainly okay with the more detailed commit message.

Ah yes. Sorry, I replied to v1 first and then realized it may have been 
easier to comment on v2.

> A few nits, though:
> 
>> On 08/02/2024 15:56, Jan Beulich wrote:
>>> On 08.02.2024 16:50, Nicola Vetrini wrote:
>>>> These 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'd like the commit message to be expanded a little bit to explain which
>> MISRA rules are a problem. This helped me to understand why we excluded
>> rather than fixed.
>>
>> Base on the previous discussion, I would suggest:
>>
>> These files contain several deliberate violation of MISRA C rules such as:
> 
> violations
> 
>>     * R20.12 on Arm for macros DEFINE and OFFSET, where the second
>> argument of OFFSET is a macro and is used as a normal parameter and a
>> stringification operand.
> 
> Is this really for Arm only?

I don't exactly know. I took Nicola's comment and massage it for the 
commit message. I am assuming that this was also not exhaustive list, so 
my aim was to only provide some example.

Thinking of it, I don't see why it would only be a problem on Arm. I can 
drop the "on Arm".

> 
>>     * R2.1 because the file is not linked That said it was decided to
>> deviate the rule itselfed to deviate that aspect).
> 
> There look to be punctuation issues here. Also s/itselfed/itself/, and
> the duplicate "deviate" is also a little odd to read (maybe "deal with"
> or "address" in place of the 2nd instance).

Doh, indeed. This wants to be:

"R2.1 because the file is not linked. That said, it was decided to 
deviate the ruule itself to address that aspect."
>> The files are also not linked in the final Xen binary, therefore they
>> can be expempted from MISRA compliance.
> 
> Looks to duplicate what the latter half of the 2nd bullet point has.
> If to be kept: s/expempted/exempted/.

I will remove.

> 
>>>> --- a/docs/misra/exclude-list.json
>>>> +++ b/docs/misra/exclude-list.json
>>>> @@ -101,6 +101,10 @@
>>>>                "rel_path": "arch/x86/efi/check.c",
>>>>                "comment": "The resulting code is not included in the final Xen binary, ignore for now"
>>>>            },
>>>> +        {
>>>> +          "rel_path": "arch/*/*/asm-offsets.c",
>>>> +          "comment": "The resulting code is not included in the final Xen binary, ignore for now"
>>>> +        },
>>>>            {
>>>>                "rel_path": "common/coverage/*",
>>>>                "comment": "Files to support gcov, ignore for now"
>>>
>>> ... something looks odd with indentation; can probably be adjusted
>>> while committing.
>>
>> I am happy to take care of both the commit message and the indentation
>> on commit.
> 
> Okay, I'll leave that to you then.

Thanks. I will do it shortly.

Cheers,

-- 
Julien Grall

Re: [XEN PATCH v2] docs/misra: add asm-offset.c to exclude-list

[Nicola Vetrini]

On 2024-02-09 13:17, Julien Grall wrote:
> Hi Jan,
> 
> On 09/02/2024 11:46, Jan Beulich wrote:
>> On 09.02.2024 10:40, Julien Grall wrote:
>>> Replying on the v2 as well.
>> 
>> And answering here despite the respective question was raised on the
>> v1 thread: I'm certainly okay with the more detailed commit message.
> 
> Ah yes. Sorry, I replied to v1 first and then realized it may have been 
> easier to comment on v2.
> 
>> A few nits, though:
>> 
>>> On 08/02/2024 15:56, Jan Beulich wrote:
>>>> On 08.02.2024 16:50, Nicola Vetrini wrote:
>>>>> These 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'd like the commit message to be expanded a little bit to explain 
>>> which
>>> MISRA rules are a problem. This helped me to understand why we 
>>> excluded
>>> rather than fixed.
>>> 
>>> Base on the previous discussion, I would suggest:
>>> 
>>> These files contain several deliberate violation of MISRA C rules 
>>> such as:
>> 
>> violations
>> 
>>>     * R20.12 on Arm for macros DEFINE and OFFSET, where the second
>>> argument of OFFSET is a macro and is used as a normal parameter and a
>>> stringification operand.
>> 
>> Is this really for Arm only?
> 
> I don't exactly know. I took Nicola's comment and massage it for the 
> commit message. I am assuming that this was also not exhaustive list, 
> so my aim was to only provide some example.
> 

Yes, indeed. I mentioned the first two examples that I remembered.

> Thinking of it, I don't see why it would only be a problem on Arm. I 
> can drop the "on Arm".
> 

My bad, I missed it in the output. Please drop the "on Arm" part.

>> 
>>>     * R2.1 because the file is not linked That said it was decided to
>>> deviate the rule itselfed to deviate that aspect).
>> 
>> There look to be punctuation issues here. Also s/itselfed/itself/, and
>> the duplicate "deviate" is also a little odd to read (maybe "deal 
>> with"
>> or "address" in place of the 2nd instance).
> 
> Doh, indeed. This wants to be:
> 
> "R2.1 because the file is not linked. That said, it was decided to 
> deviate the ruule itself to address that aspect."
                                                                          
         ^ rule
>>> The files are also not linked in the final Xen binary, therefore they
>>> can be expempted from MISRA compliance.
>> 
>> Looks to duplicate what the latter half of the 2nd bullet point has.
>> If to be kept: s/expempted/exempted/.
> 
> I will remove.
> 
>> 
>>>>> --- a/docs/misra/exclude-list.json
>>>>> +++ b/docs/misra/exclude-list.json
>>>>> @@ -101,6 +101,10 @@
>>>>>                "rel_path": "arch/x86/efi/check.c",
>>>>>                "comment": "The resulting code is not included in 
>>>>> the final Xen binary, ignore for now"
>>>>>            },
>>>>> +        {
>>>>> +          "rel_path": "arch/*/*/asm-offsets.c",
>>>>> +          "comment": "The resulting code is not included in the 
>>>>> final Xen binary, ignore for now"
>>>>> +        },
>>>>>            {
>>>>>                "rel_path": "common/coverage/*",
>>>>>                "comment": "Files to support gcov, ignore for now"
>>>> 
>>>> ... something looks odd with indentation; can probably be adjusted
>>>> while committing.
>>> 
>>> I am happy to take care of both the commit message and the 
>>> indentation
>>> on commit.
>> 
>> Okay, I'll leave that to you then.
> 
> Thanks. I will do it shortly.

Thanks,

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

Re: [XEN PATCH v2] docs/misra: add asm-offset.c to exclude-list

[Julien Grall]

Hi Nicola,

On 09/02/2024 13:20, Nicola Vetrini wrote:
> On 2024-02-09 13:17, Julien Grall wrote:
>> Hi Jan,
>>
>> On 09/02/2024 11:46, Jan Beulich wrote:
>>> On 09.02.2024 10:40, Julien Grall wrote:
>>>> Replying on the v2 as well.
>>>
>>> And answering here despite the respective question was raised on the
>>> v1 thread: I'm certainly okay with the more detailed commit message.
>>
>> Ah yes. Sorry, I replied to v1 first and then realized it may have 
>> been easier to comment on v2.
>>
>>> A few nits, though:
>>>
>>>> On 08/02/2024 15:56, Jan Beulich wrote:
>>>>> On 08.02.2024 16:50, Nicola Vetrini wrote:
>>>>>> These 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'd like the commit message to be expanded a little bit to explain 
>>>> which
>>>> MISRA rules are a problem. This helped me to understand why we excluded
>>>> rather than fixed.
>>>>
>>>> Base on the previous discussion, I would suggest:
>>>>
>>>> These files contain several deliberate violation of MISRA C rules 
>>>> such as:
>>>
>>> violations
>>>
>>>>     * R20.12 on Arm for macros DEFINE and OFFSET, where the second
>>>> argument of OFFSET is a macro and is used as a normal parameter and a
>>>> stringification operand.
>>>
>>> Is this really for Arm only?
>>
>> I don't exactly know. I took Nicola's comment and massage it for the 
>> commit message. I am assuming that this was also not exhaustive list, 
>> so my aim was to only provide some example.
>>
> 
> Yes, indeed. I mentioned the first two examples that I remembered.
> 
>> Thinking of it, I don't see why it would only be a problem on Arm. I 
>> can drop the "on Arm".
>>
> 
> My bad, I missed it in the output. Please drop the "on Arm" part.
> 
>>>
>>>>     * R2.1 because the file is not linked That said it was decided to
>>>> deviate the rule itselfed to deviate that aspect).
>>>
>>> There look to be punctuation issues here. Also s/itselfed/itself/, and
>>> the duplicate "deviate" is also a little odd to read (maybe "deal with"
>>> or "address" in place of the 2nd instance).
>>
>> Doh, indeed. This wants to be:
>>
>> "R2.1 because the file is not linked. That said, it was decided to 
>> deviate the ruule itself to address that aspect."
>          ^ rule
>>>> The files are also not linked in the final Xen binary, therefore they
>>>> can be expempted from MISRA compliance.
>>>
>>> Looks to duplicate what the latter half of the 2nd bullet point has.
>>> If to be kept: s/expempted/exempted/.
>>
>> I will remove.
>>
>>>
>>>>>> --- a/docs/misra/exclude-list.json
>>>>>> +++ b/docs/misra/exclude-list.json
>>>>>> @@ -101,6 +101,10 @@
>>>>>>                "rel_path": "arch/x86/efi/check.c",
>>>>>>                "comment": "The resulting code is not included in 
>>>>>> the final Xen binary, ignore for now"
>>>>>>            },
>>>>>> +        {
>>>>>> +          "rel_path": "arch/*/*/asm-offsets.c",
>>>>>> +          "comment": "The resulting code is not included in the 
>>>>>> final Xen binary, ignore for now"
>>>>>> +        },
>>>>>>            {
>>>>>>                "rel_path": "common/coverage/*",
>>>>>>                "comment": "Files to support gcov, ignore for now"
>>>>>
>>>>> ... something looks odd with indentation; can probably be adjusted
>>>>> while committing.
>>>>
>>>> I am happy to take care of both the commit message and the indentation
>>>> on commit.
>>>
>>> Okay, I'll leave that to you then.
>>
>> Thanks. I will do it shortly.
> 
> Thanks,

I have committed the patch now.

Cheers,

-- 
Julien Grall