Re: [XEN PATCH v2] docs/misra: add asm-offset.c to exclude-list
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
Re: [XEN PATCH v2] docs/misra: add asm-offset.c to exclude-list
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
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
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
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
Acked-by: Jan Beulich
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
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
Acked-by: Jan Beulich
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
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
Acked-by: Jan Beulich
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
