[v3] docs/misra: new rules addition

Commit Message

Stefano Stabellini June 13, 2023, 3:44 a.m. UTC

From: Stefano Stabellini <[e-mail]>

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 <[e-mail]>
---
Changes in v3:
- add all signed integer types to the Notes of 6.1
- clarify 7.2 in the Notes
- not added: marking "inapplicable" rules, to be a separate patch

Changes in v2:
- drop 5.6
- specify additional appropriate types for 6.1
---
 docs/misra/rules.rst | 51 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

Comments

Jan Beulich June 13, 2023, 7:31 a.m. UTC | #1

On 13.06.2023 05:44, Stefano Stabellini wrote:
> @@ -133,6 +146,13 @@ existing codebase are work-in-progress.
>         headers (xen/include/public/) are allowed to retain longer
>         identifiers for backward compatibility.
>  
> +   * - `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
> +     - In addition to the C99 types, we also consider appropriate types:
> +       unsigned char, unsigned short, unsigned long, unsigned long long,
> +       enum, and all explicitly signed integer types.

If I was to read this without the earlier discussion in mind, I would wonder
why the unsigned types are explicitly enumerated, but the signed ones are
described in more general terms. Can't it simply be "all explicitly unsigned
/ signed integer types", which then also covers e.g. uint32_t?

> @@ -143,6 +163,32 @@ 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
> +     - The rule asks that any integer literal that is implicitly
> +       unsigned is made explicitly unsigned by using one of the
> +       indicated suffixes.  As an example, on a machine where the int
> +       type is 32-bit wide, 0x77777777 is signed whereas 0x80000000 is
> +       (implicitly) unsigned. In order to comply with the rule, the
> +       latter should be rewritten as either 0x80000000u or 0x80000000U.
> +       Consistency considerations may suggest using the same suffix even
> +       when not required by the rule. For instance, if one has:
> +
> +       Original: f(0x77777777); f(0x80000000);
> +
> +       one might prefer
> +
> +       Solution 1: f(0x77777777U); f(0x80000000U);
> +
> +       over
> +
> +       Solution 2: f(0x77777777); f(0x80000000U);
> +
> +       after having ascertained that "Solution 1" is compatible with the
> +       intended semantics.

I think we should state here what we want people to do, not what "one
might prefer". That aspect aside, I'm not convinced the added text
(matching what Roberto did suggest) really addresses my concerns. Yet
I'm not going to pursue this any further - we'll see how this ends up
working in practice.

Jan

Stefano Stabellini June 14, 2023, 10:53 p.m. UTC | #2

On Tue, 13 Jun 2023, Jan Beulich wrote:
> On 13.06.2023 05:44, Stefano Stabellini wrote:
> > @@ -133,6 +146,13 @@ existing codebase are work-in-progress.
> >         headers (xen/include/public/) are allowed to retain longer
> >         identifiers for backward compatibility.
> >  
> > +   * - `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
> > +     - In addition to the C99 types, we also consider appropriate types:
> > +       unsigned char, unsigned short, unsigned long, unsigned long long,
> > +       enum, and all explicitly signed integer types.
> 
> If I was to read this without the earlier discussion in mind, I would wonder
> why the unsigned types are explicitly enumerated, but the signed ones are
> described in more general terms. Can't it simply be "all explicitly unsigned
> / signed integer types", which then also covers e.g. uint32_t?

I'll change it to that effect


> > @@ -143,6 +163,32 @@ 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
> > +     - The rule asks that any integer literal that is implicitly
> > +       unsigned is made explicitly unsigned by using one of the
> > +       indicated suffixes.  As an example, on a machine where the int
> > +       type is 32-bit wide, 0x77777777 is signed whereas 0x80000000 is
> > +       (implicitly) unsigned. In order to comply with the rule, the
> > +       latter should be rewritten as either 0x80000000u or 0x80000000U.
> > +       Consistency considerations may suggest using the same suffix even
> > +       when not required by the rule. For instance, if one has:
> > +
> > +       Original: f(0x77777777); f(0x80000000);
> > +
> > +       one might prefer
> > +
> > +       Solution 1: f(0x77777777U); f(0x80000000U);
> > +
> > +       over
> > +
> > +       Solution 2: f(0x77777777); f(0x80000000U);
> > +
> > +       after having ascertained that "Solution 1" is compatible with the
> > +       intended semantics.
> 
> I think we should state here what we want people to do, not what "one
> might prefer". That aspect aside, I'm not convinced the added text
> (matching what Roberto did suggest) really addresses my concerns. Yet
> I'm not going to pursue this any further - we'll see how this ends up
> working in practice.

OK. I'll keep it as is.

Patch

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index d5a6ee8cb6..f72a49c9c4 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -40,6 +40,12 @@  existing codebase are work-in-progress.
      - Summary
      - Notes
 
+   * - `Dir 1.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_01_01.c>`_
+     - Required
+     - Any implementation-defined behaviour on which the output of the
+       program depends shall be documented and understood
+     -
+
    * - `Dir 2.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_02_01.c>`_
      - Required
      - All source files shall compile without any compilation errors
@@ -57,6 +63,13 @@  existing codebase are work-in-progress.
        header file being included more than once
      -
 
+   * - `Dir 4.11 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_11.c>`_
+     - Required
+     - The validity of values passed to library functions shall be checked
+     - We do not have libraries in Xen (libfdt and others are not
+       considered libraries from MISRA C point of view as they are
+       imported in source form)
+
    * - `Dir 4.14 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_14.c>`_
      - Required
      - The validity of values received from external sources shall be
@@ -133,6 +146,13 @@  existing codebase are work-in-progress.
        headers (xen/include/public/) are allowed to retain longer
        identifiers for backward compatibility.
 
+   * - `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
+     - In addition to the C99 types, we also consider appropriate types:
+       unsigned char, unsigned short, unsigned long, unsigned long long,
+       enum, and all explicitly signed integer types.
+
    * - `Rule 6.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_02.c>`_
      - Required
      - Single-bit named bit fields shall not be of a signed type
@@ -143,6 +163,32 @@  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
+     - The rule asks that any integer literal that is implicitly
+       unsigned is made explicitly unsigned by using one of the
+       indicated suffixes.  As an example, on a machine where the int
+       type is 32-bit wide, 0x77777777 is signed whereas 0x80000000 is
+       (implicitly) unsigned. In order to comply with the rule, the
+       latter should be rewritten as either 0x80000000u or 0x80000000U.
+       Consistency considerations may suggest using the same suffix even
+       when not required by the rule. For instance, if one has:
+
+       Original: f(0x77777777); f(0x80000000);
+
+       one might prefer
+
+       Solution 1: f(0x77777777U); f(0x80000000U);
+
+       over
+
+       Solution 2: f(0x77777777); f(0x80000000U);
+
+       after having ascertained that "Solution 1" is compatible with the
+       intended semantics.
+
    * - `Rule 7.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_03.c>`_
      - Required
      - The lowercase character l shall not be used in a literal suffix
@@ -314,6 +360,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
+     -
+
    * - `Rule 22.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_22_02.c>`_
      - Mandatory
      - A block of memory shall only be freed if it was allocated by means of a