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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> 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
>