Re: [XEN PATCH] automation/eclair: extend deviations of MISRA C:2012 Rule 16.3

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.02.2024 09:53, Federico Serafini wrote:
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl

Comments below apply similarly to text added to this file.

> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -291,7 +291,14 @@ Deviations related to MISRA C:2012 Rules:
>       - Project-wide deviation; tagged as `deliberate` for ECLAIR.
>  
>     * - R16.3
> -     - Switch clauses ending with continue, goto, return statements are safe.
> +     - Switch clauses ending with an unconditional flow control statement
> +       (i.e., continue, goto, or return) are safe.
> +     - Tagged as `safe` for ECLAIR.

With this edit (unmentioned in the description, btw) ...

> +   * - R16.3
> +     - Switch clauses ending with an if-else statement are safe if both
> +       branches consist of a flow control statement (i.e., continue, break,
> +       goto, return).

... why is it not also "ending with" here?

Also what about either situation ending with a call to a noreturn function?

> @@ -307,6 +314,16 @@ Deviations related to MISRA C:2012 Rules:
>       - Switch clauses ending with failure method \"BUG()\" are safe.
>       - Tagged as `safe` for ECLAIR.
>  
> +   * - R16.3
> +     - On X86, switch clauses ending generating an exception through
> +       \"generate_exception()\" are safe.
> +     - Tagged as `safe` for ECLAIR.

This macro is limited to the emulator, so shouldn't be deviated globally.
Furthermore - why does the special case need mentioning here? Shouldn't
it be the underlying pattern which is deviated (along the lines of the
earlier ones):

    if ( true )
    {
        ...
        goto ...; /* Or break / continue / return */
    }

> +   * - R16.3
> +     - Switch clauses ending generating a parse error through
> +       \"PARSE_ERR_RET()\" are safe.
> +     - Tagged as `safe` for ECLAIR.

Again this isn't a global scope macro, so shouldn't be deviated globally.
Plus it ends in "return", so ought to be covered by the earlier clause.
The fact that the return is in a body of do {} while(0) shouldn't matter
at all - that's purely syntactic sugar.

Jan