[XEN PATCH v3 0/2] address violations of MISRA C Rule 16.2

[XEN PATCH v3 0/2] address violations of MISRA C Rule 16.2

[Nicola Vetrini]

A respin of the last two patches from the previous series.
No changes other than the different SAF comment id because SAF-4-safe was
already taken.

Nicola Vetrini (2):
  xen/domctl: address violations of MISRA C Rule 16.2
  eclair_analysis: deviate x86 emulator for Rule 16.2

 automation/eclair_analysis/ECLAIR/deviations.ecl | 7 +++++++
 docs/misra/deviations.rst                        | 6 ++++++
 docs/misra/safe.json                             | 8 ++++++++
 xen/common/domain.c                              | 1 +
 4 files changed, 22 insertions(+)

-- 
2.34.1

[XEN PATCH v3 1/2] xen/domctl: address violations of MISRA C Rule 16.2

[Nicola Vetrini]

Refactor the first clauses so that a violation of
MISRA C Rule 16.2 is resolved (a switch label should be immediately
enclosed in the compound statement of the switch).
Note that the switch clause ending with the pseudo
keyword "fallthrough" is an allowed exception to Rule 16.3.

Convert fallthrough comments in other clauses to the pseudo-keyword
while at it.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 docs/misra/safe.json | 8 ++++++++
 xen/common/domain.c  | 1 +
 2 files changed, 9 insertions(+)

diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index fe2bc185097d..9b13bcf71706 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -44,6 +44,14 @@
         },
         {
             "id": "SAF-5-safe",
+            "analyser": {
+                "eclair": "MC3R1.R16.2"
+            },
+            "name": "MC3R1.R16.2: using a case label when the most closely-enclosing compound statement is not a switch statement",
+            "text": "A switch label enclosed by some compound statement that is not the body of a switch is permitted within local helper macros that are unlikely to be misused or misunderstood."
+        },
+        {
+            "id": "SAF-6-safe",
             "analyser": {},
             "name": "Sentinel",
             "text": "Next ID to be used"
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 282c3ab62308..1e555d658c97 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -457,6 +457,7 @@ static int domain_teardown(struct domain *d)
 
         for_each_vcpu ( d, v )
         {
+            /* SAF-5-safe MISRA C Rule 16.2: switch label enclosed by for loop*/
             PROGRESS_VCPU(teardown);
 
             rc = vcpu_teardown(v);
-- 
2.34.1

[XEN PATCH v3 2/2] eclair_analysis: deviate x86 emulator for Rule 16.2

[Nicola Vetrini]

MISRA C Rule 16.2 states:
"A switch label shall only be used when the most closely-enclosing
compound statement is the body of a switch statement".

Since complying with this rule of the x86 emulator would lead to
a lot of code duplication, it is deemed better to exempt those
files for this guideline.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 7 +++++++
 docs/misra/deviations.rst                        | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 0230b41c6d1c..190f6a2fd4e0 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -358,6 +358,13 @@ therefore have the same behavior of a boolean"
 # Series 16.
 #
 
+-doc_begin="Complying with the Rule would entail a lot of code duplication in the implementation of the x86 emulator,
+therefore it is deemed better to leave such files as is."
+-file_tag+={x86_emulate,"^xen/arch/x86/x86_emulate/.*$"}
+-file_tag+={x86_svm_emulate,"^xen/arch/x86/hvm/svm/emulate\\.c$"}
+-config=MC3R1.R16.2,reports+={deliberate, "any_area(any_loc(file(x86_emulate||x86_svm_emulate)))"}
+-doc_end
+
 -doc_begin="Switch clauses ending with continue, goto, return statements are
 safe."
 -config=MC3R1.R16.3,terminals+={safe, "node(continue_stmt||goto_stmt||return_stmt)"}
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 32b02905d19a..ed0c1e8ed0bf 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -296,6 +296,12 @@ Deviations related to MISRA C:2012 Rules:
        therefore have the same behavior of a boolean.
      - Project-wide deviation; tagged as `deliberate` for ECLAIR.
 
+   * - R16.2
+     - Complying with the Rule would entail a lot of code duplication in the
+       implementation of the x86 emulator, therefore it is deemed better to
+       leave such files as is.
+     - Tagged as `deliberate` for ECLAIR.
+
    * - R16.3
      - Switch clauses ending with continue, goto, return statements are safe.
      - Tagged as `safe` for ECLAIR.
-- 
2.34.1

Re: [XEN PATCH v3 1/2] xen/domctl: address violations of MISRA C Rule 16.2

[Jan Beulich]

On 17.04.2024 21:37, Nicola Vetrini wrote:
> Refactor the first clauses so that a violation of
> MISRA C Rule 16.2 is resolved (a switch label should be immediately
> enclosed in the compound statement of the switch).
> Note that the switch clause ending with the pseudo
> keyword "fallthrough" is an allowed exception to Rule 16.3.
> 
> Convert fallthrough comments in other clauses to the pseudo-keyword
> while at it.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

I did ack a patch with this title, yes, but the content of that patch
was different. Besides all the references to the fallthrough pseudo-
keyword being unrelated to the change below, there's also nothing ...

> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -457,6 +457,7 @@ static int domain_teardown(struct domain *d)
>  
>          for_each_vcpu ( d, v )
>          {
> +            /* SAF-5-safe MISRA C Rule 16.2: switch label enclosed by for loop*/
>              PROGRESS_VCPU(teardown);
>  
>              rc = vcpu_teardown(v);

... domctl-ish in here. What is going on?

Jan

Re: [XEN PATCH v3 1/2] xen/domctl: address violations of MISRA C Rule 16.2

[Nicola Vetrini]

On 2024-04-18 17:10, Jan Beulich wrote:
> On 17.04.2024 21:37, Nicola Vetrini wrote:
>> Refactor the first clauses so that a violation of
>> MISRA C Rule 16.2 is resolved (a switch label should be immediately
>> enclosed in the compound statement of the switch).
>> Note that the switch clause ending with the pseudo
>> keyword "fallthrough" is an allowed exception to Rule 16.3.
>> 
>> Convert fallthrough comments in other clauses to the pseudo-keyword
>> while at it.
>> 
>> No functional change.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> I did ack a patch with this title, yes, but the content of that patch
> was different. Besides all the references to the fallthrough pseudo-
> keyword being unrelated to the change below, there's also nothing ...
> 
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -457,6 +457,7 @@ static int domain_teardown(struct domain *d)
>> 
>>          for_each_vcpu ( d, v )
>>          {
>> +            /* SAF-5-safe MISRA C Rule 16.2: switch label enclosed by 
>> for loop*/
>>              PROGRESS_VCPU(teardown);
>> 
>>              rc = vcpu_teardown(v);
> 
> ... domctl-ish in here. What is going on?
> 
> Jan

Oh, I must have edited the wrong patch when doing v3. Sorry about that.
The patch was this [1], just with the id changed.

[1] 
https://lore.kernel.org/xen-devel/2328b1a43f8bc4c9a9303c46f8eba2847c67a713.1712305581.git.nicola.vetrini@bugseng.com/

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