Re: [RFC PATCH 01/18] arm: cppcheck: misra rule 20.7 deviations for alternative.h

[Julien Grall <julien@xxxxxxx>]

Hi Luca,

On 20/12/2022 08:50, Luca Fancellu wrote:
> Cppcheck reports violations of rule 20.7 in two macros of
> alternative.h.
>
> The first one is in the __ALT_PTR macro where cppcheck wants to have
> parentheses on the second operand of a member access operator, which
> is not allowed from the c language syntax, furthermore, the macro
> parameter is never used as an expression, hence we can suppress the
> finding.
Looking at the Misra Rule 20.7 examples [1], this looks similar to 
GET_MEMBER(). So I don't understand why cppcheck is complaining.
> The second finding is in the __ALTERNATIVE_CFG macro, where cppcheck
> wants to have parentheses on the macro arguments, but the macro
> parameters are never used as expressions and are used only for text
> substitution, so cppcheck is not taking into account the context of
> use of the macro arguments and adding parenthesis would break the
> code, so we can suppress the finding.
This reads like you want to report a bug against cppcheck.

> No error was shown by eclair and coverity for those lines.
>
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
>   docs/misra/false-positive-cppcheck.json | 14 ++++++++++++++
>   xen/arch/arm/include/asm/alternative.h  |  2 ++
This file is meant to be close to Linux. From my understanding of the 
discussion yesterday, we didn't want to add deviation there.
>   2 files changed, 16 insertions(+)
>
> diff --git a/docs/misra/false-positive-cppcheck.json 
> b/docs/misra/false-positive-cppcheck.json
> index 5d4da2ce6170..5e7d9377f60b 100644
> --- a/docs/misra/false-positive-cppcheck.json
> +++ b/docs/misra/false-positive-cppcheck.json
> @@ -3,6 +3,20 @@
>       "content": [
>           {
>               "id": "SAF-0-false-positive-cppcheck",
> +            "violation-id": "misra-c2012-20.7",
> +            "tool-version": "2.7",
> +            "name": "R20.7 second operand of member-access operator",
> +            "text": "The second operand of a member access operator shall be a name 
> of a member of the type pointed to, so in this particular case it is wrong to use parentheses on 
> the macro parameter."
> +        },
> +        {
> +            "id": "SAF-1-false-positive-cppcheck",
> +            "violation-id": "misra-c2012-20.7",
> +            "tool-version": "2.7",
> +            "name": "R20.7 C macro parameters used only for text substitution",
> +            "text": "The macro parameters used in this case are not part of an 
> expression, they are used for text substitution."
> +        },
In both cases the constructs are commonly used in Xen to generate code. 
So I am a bit concerned to have to add deviation everywhere cppcheck is 
wrong.
More generally, we are still at the beginning of MISRA in Xen and I 
don't think we should start with adding deviations from bugs in the 
tools. Instead, we should report those bugs and hopefully by the time 
Xen is mostly MISRA complaint the tools will not report the false positive.
If they are still then we can decide to add deviations.

Cheers,

[1] 
https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_20_07.c
--
Julien Grall