Top MISRA violations

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>]

Hi all,

Francesco kindly shared his spreadsheet with the analysis of the MISRA
violations, see attached. I am sharing it here to the list as all the
members of fuse-sig have access to MISRA. I also want to take this
opportunity to thank Francesco and his team for their work.


Looking at the spreadsheet, the top required rules with more than 1000
violations are: 15.6, 21.2, 5.1, 8.4.


# Rule 15.6

The body of an iteration-statement (while, for) or a selection-statement
(if, switch) shall be a compound statement.

For example:

    /* non compliant */
    if ( x )
        func();
    
    /* compliant */
    if ( x )
    {
        func();
    }

Where the real issue are cases like this:

    if ( one )
        if ( two )
            foo();
        else
            bar();


## Analysis & Potential Solution

There are many places in the Xen code where we skip the curly brackets
when the statement under 'if' is a one-liner. A solution that was
suggested before was to use GCC automatic checking for these situations:

    -Wmisleading-indentation (C and C++ only)
              Warn when the indentation of the code does not reflect the block
              structure.  Specifically, a warning is issued for "if", "else",
              "while", and "for" clauses with a guarded statement that does not
              use braces, followed by an unguarded statement with the same
              indentation.
   
              In the following example, the call to "bar" is misleadingly
              indented as if it were guarded by the "if" conditional.
   
                        if (some_condition ())
                          foo ();
                          bar ();  /* Gotcha: this is not guarded by the "if".  
*/

              [...]

We could document that -Wmisleading-indentation is required and should
be used as a check against this kind of issues. A patch to the Xen
Makefile to add -Wmisleading-indentation by default would also help. In
this context, we would be using GCC as a static code checker, not as a
compiler (GCC is not a safety certified compiler.)


# Rule 21.2

A reserved identifier or macro name shall not be declared.

Example, non-compliant:

    extern void *memcpy(void *restrict s1, const void *restrict s2, size_t n);

It is non-compliant because we are meant to #include <string.h> to get
the declaration of memcpy.


## Analysis & Potential Solution

In a kernel or a hypervisor it is not possible to #include <string.h>
because the standard library is not available.

Probably it is just a matter of documenting this deviation?


# Rule 5.1

External identifiers shall be distinct.

/* non compliant */
int32_t abc = 0;
int32_t ABC = 0;

/* non compliant: 31 significant chars are common */
int32_t engine_exhaust_gas_temperature_raw;
int32_t engine_exhaust_gas_temperature_scaled;

- in C90 the minimum requirement is that the first 6 characters of
  external identifiers are significant but their case is not required to
  be significant;
- in C99 the minimum requirement is that the first 31 characters of
  external identifiers are significant, with each universal character or
  corresponding extended source character occupying between 6 and 10
  characters. 


## Analysis & Potential Solution

In reality most implementations provide far greater limits. It is common
for external identifiers in C90 to be case-sensitive and for the first
31 characters to be significant.

Should we document that we require a C compiler that supports at least
the first 31 characters to be significant and case-sensitive? Then
configure the MISRA checker accordingly. Would it be good enough?


# Rule 8.4

A compatible declaration shall be visible when an object or function
with external linkage is defined.

Example:

extern int16_t count;
int16_t count = 1; /* compliant */

extern int16_t count = 1; /* non-compliant */


## Analysis & Potential Solution

This is not a rule I would expect Xen to be violating. I wonder how it
made the list. I grepped for extern variables that are assigning an
initial value at the time of declaration but couldn't find any.
Interestingly, I also found an old email from Lars where he believed
that this kind of violations in Xen where false positive.

Francesco, it would be fantastic if you could provide a couple of
examples so that we can take a closer look.

And thanks again for your help on this!

Cheers,

Stefano

Attachment: CodecheckResultXEN0.5.xlsx
Description: MS-Excel 2007 spreadsheet