Possible false positive for MISRA Rule 20.7

[khouri]

I noticed a possible false positive for MISRA rule 20.7 in a large project and created a simplified snippet to exemplify the situation, see below.

typedef struct {
    int sail_size;
} BOAT;

static BOAT  my_boat;

/* Rule 20.7 violation is reported on this line */
#define ACCESS_MEMBER(member)    my_boat.member

int main(void)
{
    ACCESS_MEMBER(sail_size);
    return 0;
}

This yields as output:

$ gcc -c cppcheck_misra20-7.c && cppcheck --enable=all --addon=misra.py cppcheck_misra20-7.c
Checking cppcheck_misra20-7.c ...
cppcheck_misra20-7.c:9:0: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-20.7]
#define ACCESS_MEMBER(member)    my_boat.member

Given the example on page 170 the the MISRA C:2012 standard, I believe this would be a valid macro and thus a false positive. Replacing my_boat.member with my_boat.(member) in the macro definition would not be valid C code.

As a closing note: I really appreciate the MISRA support now being offered by Cppcheck and the hard work which must have gone into this. I only discovered the recent series of kickstarters for expanded MISRA C support in Cppcheck. Should there be another one, I would certainly be among the backers.

[Georgiy Komarov]

Thanks! I created this ticket: https://trac.cppcheck.net/ticket/9673

[Amir]

I've pulled the lastest misra.py and this doesn't seem to be resolved yet. I'm also getting this for the following macro:

#define SelectMuxChannel(channel)           \
    (GPIO2_SetFieldValue(GPIO2_DeviceData, PORTH2, (channel & MUX_SELECT_MASK)))

[Georgiy Komarov]

This seems correct to me.

In your example channel should be enclosed in parentheses. Otherwise, if the operator precedence was broken, macro substitution may lead to an unexpected result.

Consider for example the following call: SelectMuxChannel(0x10 | 0x20). We'll get the following code in substitution: 0x10 | 0x20 & MUX_SELECT_MASK. Since the & operator has a higher precedence, this code will be executed as 0x10 | (0x20 & MUX_SELECT_MASK). This is definitely not what the user expects.

MISRA-compliant code will look like this:

#define SelectMuxChannel(channel)           \
    (GPIO2_SetFieldValue(GPIO2_DeviceData, PORTH2, ((channel) & MUX_SELECT_MASK)))

[Amir]

my mistake, sorry about that!

[Georgiy Komarov]

No problem, thanks for the report