Re: [PATCH] docs/misra: new rules addition

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.06.2023 03:38, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> 
> For Dir 1.1, a document describing all implementation-defined behaviour
> (i.e. gcc-specific behavior) will be added to docs/misra, also including
> implementation-specific (gcc-specific) appropriate types for bit-field
> relevant to Rule 6.1.
> 
> Rule 21.21 is lacking an example on gitlab but the rule is
> straightforward: we don't use stdlib at all in Xen.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> ---
> We also discussed Rules 2.1, 5.5 and 7.4 but they require more work
> before we can decide one way or the other.

Since I wasn't able to attend yesterday's meeting, please forgive a
couple of remarks:

> @@ -133,6 +146,16 @@ existing codebase are work-in-progress.
>         headers (xen/include/public/) are allowed to retain longer
>         identifiers for backward compatibility.
>  
> +   * - `Rule 5.6 
> <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_05_06.c>`_
> +     - Required
> +     - A typedef name shall be a unique identifier
> +     -

Considering that the rule requires uniqueness across the entire code
base (and hence precludes e.g. two functions having identically named
local typedefs), I'm a little puzzled this was adopted. I for one
question that a use like the one mentioned is really at risk of being
confusing. Instead I think that the need to change at least one of
the names is at risk of making the code less readable then, even if
ever so slightly. (All of this said - I don't know if we have any
violations of this rule.)

> +   * - `Rule 6.1 
> <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_01.c>`_
> +     - Required
> +     - Bit-fields shall only be declared with an appropriate type
> +     -

This requires you have settled on what "an appropriate type" is, i.e.
whether our uses of e.g. types wider than int is meant to become a
deviation, or will need eliminating. I suppose the outcome of this
could do with mentioning as a remark here.

> @@ -143,6 +166,12 @@ existing codebase are work-in-progress.
>       - Octal constants shall not be used
>       -
>  
> +   * - `Rule 7.2 
> <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_
> +     - Required
> +     - A "u" or "U" suffix shall be applied to all integer constants
> +       that are represented in an unsigned type
> +     -

"Represented in an unsigned type" means there is a dependency on the
target architecture and compiler capabilities: Representation can only
be determined once knowing the underlying binary ABI, and uses in #if
and alike require knowing the widest integer type's size that the
compiler supports. As a consequence this looks like it may require, in
certain cases, to add u/U conditionally. Any such conditionals pose a
risk of cluttering the code.

> @@ -314,6 +343,11 @@ existing codebase are work-in-progress.
>         used following a subsequent call to the same function
>       -
>  
> +   * - Rule 21.21
> +     - Required
> +     - The Standard Library function system of <stdlib.h> shall not be used
> +     -

I wish inapplicable (to us) rules would also be marked as such.

Jan