diff mbox series

[XEN,v2,4/6,RESEND] automation/eclair_analysis: address violations of MISRA C Rule 20.7

Message ID dfebde9cc657f2669df60b08ca34352288e082ab.1718378539.git.nicola.vetrini@bugseng.com (mailing list archive)
State New
Headers show
Series address violations of MISRA C Rule 20.7 | expand

Commit Message

Nicola Vetrini June 17, 2024, 8:57 a.m. UTC
MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses".

The local helpers GRP2 and XADD in the x86 emulator use their first
argument as the constant expression for a case label. This pattern
is deviated project-wide, because it is very unlikely to induce
developer confusion and result in the wrong control flow being
carried out.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Changes in v2:
- Introduce a deviation instead of adding parentheses
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++++--
 docs/misra/deviations.rst                        | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

Comments

Stefano Stabellini June 21, 2024, 12:20 a.m. UTC | #1
On Mon, 17 Jun 2024, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses".
> 
> The local helpers GRP2 and XADD in the x86 emulator use their first
> argument as the constant expression for a case label. This pattern
> is deviated project-wide, because it is very unlikely to induce
> developer confusion and result in the wrong control flow being
> carried out.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Stefano Stabellini June 25, 2024, 12:45 a.m. UTC | #2
On Mon, 17 Jun 2024, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses".
> 
> The local helpers GRP2 and XADD in the x86 emulator use their first
> argument as the constant expression for a case label. This pattern
> is deviated project-wide, because it is very unlikely to induce
> developer confusion and result in the wrong control flow being
> carried out.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes in v2:
> - Introduce a deviation instead of adding parentheses
> ---
>  automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++++--
>  docs/misra/deviations.rst                        | 3 ++-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index c2698e7074aa..fc248641dc78 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -428,13 +428,15 @@ unexpected result when the structure is given as argument to a sizeof() operator
>  
>  -doc_begin="Code violating Rule 20.7 is safe when macro parameters are used: (1)
>  as function arguments; (2) as macro arguments; (3) as array indices; (4) as lhs
> -in assignments; (5) as initializers, possibly designated, in initalizer lists."
> +in assignments; (5) as initializers, possibly designated, in initalizer lists;
> +(6) as the constant expression in a switch clause label."
>  -config=MC3R1.R20.7,expansion_context=
>  {safe, "context(__call_expr_arg_contexts)"},
>  {safe, "left_right(^[(,\\[]$,^[),\\]]$)"},
>  {safe, "context(skip_to(__expr_non_syntactic_contexts, stmt_child(node(array_subscript_expr), subscript)))"},
>  {safe, "context(skip_to(__expr_non_syntactic_contexts, stmt_child(operator(assign), lhs)))"},
> -{safe, "context(skip_to(__expr_non_syntactic_contexts, stmt_child(node(init_list_expr||designated_init_expr), init)))"}
> +{safe, "context(skip_to(__expr_non_syntactic_contexts, stmt_child(node(init_list_expr||designated_init_expr), init)))"},
> +{safe, "context(skip_to(__expr_non_syntactic_contexts, stmt_child(node(case_stmt), lower||upper)))"}
>  -doc_end
>  
>  -doc_begin="Violations involving the __config_enabled macros cannot be fixed without
> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> index 36959aa44ac9..be2cc6bf03eb 100644
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -376,7 +376,8 @@ Deviations related to MISRA C:2012 Rules:
>         (2) as macro arguments;
>         (3) as array indices;
>         (4) as lhs in assignments;
> -       (5) as initializers, possibly designated, in initalizer lists.
> +       (5) as initializers, possibly designated, in initalizer lists;
> +       (6) as constant expressions of switch case labels.
>       - Tagged as `safe` for ECLAIR.
>  
>     * - R20.7
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index c2698e7074aa..fc248641dc78 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -428,13 +428,15 @@  unexpected result when the structure is given as argument to a sizeof() operator
 
 -doc_begin="Code violating Rule 20.7 is safe when macro parameters are used: (1)
 as function arguments; (2) as macro arguments; (3) as array indices; (4) as lhs
-in assignments; (5) as initializers, possibly designated, in initalizer lists."
+in assignments; (5) as initializers, possibly designated, in initalizer lists;
+(6) as the constant expression in a switch clause label."
 -config=MC3R1.R20.7,expansion_context=
 {safe, "context(__call_expr_arg_contexts)"},
 {safe, "left_right(^[(,\\[]$,^[),\\]]$)"},
 {safe, "context(skip_to(__expr_non_syntactic_contexts, stmt_child(node(array_subscript_expr), subscript)))"},
 {safe, "context(skip_to(__expr_non_syntactic_contexts, stmt_child(operator(assign), lhs)))"},
-{safe, "context(skip_to(__expr_non_syntactic_contexts, stmt_child(node(init_list_expr||designated_init_expr), init)))"}
+{safe, "context(skip_to(__expr_non_syntactic_contexts, stmt_child(node(init_list_expr||designated_init_expr), init)))"},
+{safe, "context(skip_to(__expr_non_syntactic_contexts, stmt_child(node(case_stmt), lower||upper)))"}
 -doc_end
 
 -doc_begin="Violations involving the __config_enabled macros cannot be fixed without
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 36959aa44ac9..be2cc6bf03eb 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -376,7 +376,8 @@  Deviations related to MISRA C:2012 Rules:
        (2) as macro arguments;
        (3) as array indices;
        (4) as lhs in assignments;
-       (5) as initializers, possibly designated, in initalizer lists.
+       (5) as initializers, possibly designated, in initalizer lists;
+       (6) as constant expressions of switch case labels.
      - Tagged as `safe` for ECLAIR.
 
    * - R20.7