[PATCH v2] docs/misra: add 14.3

[PATCH v2] docs/misra: add 14.3

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@amd.com>

Add 14.3, with project-wide deviations.

Also take the opportunity to clarify that parameters of function pointer
types are expected to have names (Rule 8.2).

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
v2:
- remove 14.4
- better wording for the 8.2 clarification
- add while(0) and while(1) to 14.3
- add ?: to the deviation
- remove list of statements the rule applies to
---
 docs/misra/rules.rst | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index db30632b93..9389976290 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -234,7 +234,8 @@ maintainers if you want to suggest a change.
    * - `Rule 8.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_02.c>`_
      - Required
      - Function types shall be in prototype form with named parameters
-     -
+     - Clarification: both function and function pointers types shall
+       have named parameters.
 
    * - `Rule 8.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_03.c>`_
      - Required
@@ -332,6 +333,17 @@ maintainers if you want to suggest a change.
      - A loop counter shall not have essentially floating type
      -
 
+   * - `Rule 14.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_03.c>`_
+     - Required
+     - Controlling expressions shall not be invariant
+     - Due to the extensive usage of IS_ENABLED, sizeof compile-time
+       checks, and other constructs that are detected as errors by MISRA
+       C scanners, managing the configuration of a MISRA C scanner for
+       this rule would be unmanageable. Thus, this rule is adopted with
+       a project-wide deviation on if and ?: statements.
+
+       while(0) and while(1) are allowed.
+
    * - `Rule 16.7 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_16_07.c>`_
      - Required
      - A switch-expression shall not have essentially Boolean type
-- 
2.25.1

Re: [PATCH v2] docs/misra: add 14.3

[Jan Beulich]

On 31.08.2023 03:53, Stefano Stabellini wrote:
> @@ -332,6 +333,17 @@ maintainers if you want to suggest a change.
>       - A loop counter shall not have essentially floating type
>       -
>  
> +   * - `Rule 14.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_03.c>`_
> +     - Required
> +     - Controlling expressions shall not be invariant
> +     - Due to the extensive usage of IS_ENABLED, sizeof compile-time
> +       checks, and other constructs that are detected as errors by MISRA
> +       C scanners, managing the configuration of a MISRA C scanner for
> +       this rule would be unmanageable. Thus, this rule is adopted with
> +       a project-wide deviation on if and ?: statements.

As before - switch(sizeof(...)) and alike also wants deviating, imo.

> +       while(0) and while(1) are allowed.

... and alike ... (to also cover e.g. while(true)).

Jan

Re: [PATCH v2] docs/misra: add 14.3

[Stefano Stabellini]

On Thu, 31 Aug 2023, Jan Beulich wrote:
> On 31.08.2023 03:53, Stefano Stabellini wrote:
> > @@ -332,6 +333,17 @@ maintainers if you want to suggest a change.
> >       - A loop counter shall not have essentially floating type
> >       -
> >  
> > +   * - `Rule 14.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_03.c>`_
> > +     - Required
> > +     - Controlling expressions shall not be invariant
> > +     - Due to the extensive usage of IS_ENABLED, sizeof compile-time
> > +       checks, and other constructs that are detected as errors by MISRA
> > +       C scanners, managing the configuration of a MISRA C scanner for
> > +       this rule would be unmanageable. Thus, this rule is adopted with
> > +       a project-wide deviation on if and ?: statements.
> 
> As before - switch(sizeof(...)) and alike also wants deviating, imo.

I added the switch, but without the words "and alike" because it is not
clear in this context what it would allow.


> > +       while(0) and while(1) are allowed.
> 
> ... and alike ... (to also cover e.g. while(true)).

Here the words "and alike" are clearer so I added them

[PATCH v2] docs/misra: add 14.3

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@amd.com>

Add 14.3, with project-wide deviations.

Also take the opportunity to clarify that parameters of function pointer
types are expected to have names (Rule 8.2).

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
Changes in v2:
- add switch to the Rule 14.3 exceptions
- add "and alike" for while(0) and while(1)
---
 docs/misra/rules.rst | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 34916e266a..3d8d1633a4 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -234,7 +234,8 @@ maintainers if you want to suggest a change.
    * - `Rule 8.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_02.c>`_
      - Required
      - Function types shall be in prototype form with named parameters
-     -
+     - Clarification: both function and function pointers types shall
+       have named parameters.
 
    * - `Rule 8.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_03.c>`_
      - Required
@@ -385,6 +386,17 @@ maintainers if you want to suggest a change.
      - A loop counter shall not have essentially floating type
      -
 
+   * - `Rule 14.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_03.c>`_
+     - Required
+     - Controlling expressions shall not be invariant
+     - Due to the extensive usage of IS_ENABLED, sizeof compile-time
+       checks, and other constructs that are detected as errors by MISRA
+       C scanners, managing the configuration of a MISRA C scanner for
+       this rule would be unmanageable. Thus, this rule is adopted with
+       a project-wide deviation on if ?: and switch statements.
+
+       while(0) and while(1) and alike are allowed.
+
    * - `Rule 16.7 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_16_07.c>`_
      - Required
      - A switch-expression shall not have essentially Boolean type
-- 
2.25.1

Re: [PATCH v2] docs/misra: add 14.3

[Jan Beulich]

On 07.09.2023 03:21, Stefano Stabellini wrote:
> On Thu, 31 Aug 2023, Jan Beulich wrote:
>> On 31.08.2023 03:53, Stefano Stabellini wrote:
>>> @@ -332,6 +333,17 @@ maintainers if you want to suggest a change.
>>>       - A loop counter shall not have essentially floating type
>>>       -
>>>  
>>> +   * - `Rule 14.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_03.c>`_
>>> +     - Required
>>> +     - Controlling expressions shall not be invariant
>>> +     - Due to the extensive usage of IS_ENABLED, sizeof compile-time
>>> +       checks, and other constructs that are detected as errors by MISRA
>>> +       C scanners, managing the configuration of a MISRA C scanner for
>>> +       this rule would be unmanageable. Thus, this rule is adopted with
>>> +       a project-wide deviation on if and ?: statements.
>>
>> As before - switch(sizeof(...)) and alike also wants deviating, imo.
> 
> I added the switch, but without the words "and alike" because it is not
> clear in this context what it would allow.

Well, what I mean here are expressions involving sizeof() (or e.g also
offsetof()).

Jan

Re: [PATCH v2] docs/misra: add 14.3

[Jan Beulich]

On 07.09.2023 03:22, Stefano Stabellini wrote:
> @@ -385,6 +386,17 @@ maintainers if you want to suggest a change.
>       - A loop counter shall not have essentially floating type
>       -
>  
> +   * - `Rule 14.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_03.c>`_
> +     - Required
> +     - Controlling expressions shall not be invariant
> +     - Due to the extensive usage of IS_ENABLED, sizeof compile-time
> +       checks, and other constructs that are detected as errors by MISRA
> +       C scanners, managing the configuration of a MISRA C scanner for
> +       this rule would be unmanageable. Thus, this rule is adopted with
> +       a project-wide deviation on if ?: and switch statements.

Do we want to go as far as permitting this uniformly for all switch()? In
my earlier reply I had included sizeof() for a reason.

Also (nit) there's at least a comma missing after "if". To make clear it's
keywords that are meant, maybe better use if() / switch()?

Jan

Re: [PATCH v2] docs/misra: add 14.3

[Stefano Stabellini]

On Thu, 7 Sep 2023, Jan Beulich wrote:
> On 07.09.2023 03:22, Stefano Stabellini wrote:
> > @@ -385,6 +386,17 @@ maintainers if you want to suggest a change.
> >       - A loop counter shall not have essentially floating type
> >       -
> >  
> > +   * - `Rule 14.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_03.c>`_
> > +     - Required
> > +     - Controlling expressions shall not be invariant
> > +     - Due to the extensive usage of IS_ENABLED, sizeof compile-time
> > +       checks, and other constructs that are detected as errors by MISRA
> > +       C scanners, managing the configuration of a MISRA C scanner for
> > +       this rule would be unmanageable. Thus, this rule is adopted with
> > +       a project-wide deviation on if ?: and switch statements.
> 
> Do we want to go as far as permitting this uniformly for all switch()? In
> my earlier reply I had included sizeof() for a reason.
 
I agree with you that it would be better to restrict it to only some
switch uses, rather than all of them.

But if we are going to restrict the deviation to switch(sizeof()), which
I think is a good idea and I am in favor, wouldn't it be better to
handle these cases as individual deviations? E.g. docs/misra/safe.json?
I am assuming there are only few cases like that and adding it here
makes the rule more complicated.

I am happy either way but I wanted to provide that as an option.


> Also (nit) there's at least a comma missing after "if". To make clear it's
> keywords that are meant, maybe better use if() / switch()?

OK I'll do that

Re: [PATCH v2] docs/misra: add 14.3

[Jan Beulich]

On 07.09.2023 23:45, Stefano Stabellini wrote:
> On Thu, 7 Sep 2023, Jan Beulich wrote:
>> On 07.09.2023 03:22, Stefano Stabellini wrote:
>>> @@ -385,6 +386,17 @@ maintainers if you want to suggest a change.
>>>       - A loop counter shall not have essentially floating type
>>>       -
>>>  
>>> +   * - `Rule 14.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_03.c>`_
>>> +     - Required
>>> +     - Controlling expressions shall not be invariant
>>> +     - Due to the extensive usage of IS_ENABLED, sizeof compile-time
>>> +       checks, and other constructs that are detected as errors by MISRA
>>> +       C scanners, managing the configuration of a MISRA C scanner for
>>> +       this rule would be unmanageable. Thus, this rule is adopted with
>>> +       a project-wide deviation on if ?: and switch statements.
>>
>> Do we want to go as far as permitting this uniformly for all switch()? In
>> my earlier reply I had included sizeof() for a reason.
>  
> I agree with you that it would be better to restrict it to only some
> switch uses, rather than all of them.
> 
> But if we are going to restrict the deviation to switch(sizeof()), which
> I think is a good idea and I am in favor, wouldn't it be better to
> handle these cases as individual deviations? E.g. docs/misra/safe.json?
> I am assuming there are only few cases like that and adding it here
> makes the rule more complicated.

Personally I think it wants to be both anyway. For one, anything written
here still needs respective SAF annotations for scanners to be uniformly
aware (dealing with deviations in just the Eclair configuration is imo
dubious). And then my general view is that by stating patterns here we
make clear that we tolerate new instances of such constructs, whereas in
other cases we'd be aiming at no deviations in new code.

Jan

Re: [PATCH v2] docs/misra: add 14.3

[Nicola Vetrini]

On 07/09/2023 23:45, Stefano Stabellini wrote:
> On Thu, 7 Sep 2023, Jan Beulich wrote:
>> On 07.09.2023 03:22, Stefano Stabellini wrote:
>> > @@ -385,6 +386,17 @@ maintainers if you want to suggest a change.
>> >       - A loop counter shall not have essentially floating type
>> >       -
>> >
>> > +   * - `Rule 14.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_03.c>`_
>> > +     - Required
>> > +     - Controlling expressions shall not be invariant
>> > +     - Due to the extensive usage of IS_ENABLED, sizeof compile-time
>> > +       checks, and other constructs that are detected as errors by MISRA
>> > +       C scanners, managing the configuration of a MISRA C scanner for
>> > +       this rule would be unmanageable. Thus, this rule is adopted with
>> > +       a project-wide deviation on if ?: and switch statements.
>> 
>> Do we want to go as far as permitting this uniformly for all switch()? 
>> In
>> my earlier reply I had included sizeof() for a reason.
> 
> I agree with you that it would be better to restrict it to only some
> switch uses, rather than all of them.
> 
> But if we are going to restrict the deviation to switch(sizeof()), 
> which
> I think is a good idea and I am in favor, wouldn't it be better to
> handle these cases as individual deviations? E.g. docs/misra/safe.json?
> I am assuming there are only few cases like that and adding it here
> makes the rule more complicated.
> 
> I am happy either way but I wanted to provide that as an option.
> 
> 

It's also worth considering the magnitude of required SAF comments when 
making this choice.
In this case, from a cursory glance at the present violations, it seems 
that there are maybe a
handful of switches inside macros, so they shouldn't be problematic to 
deal with.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [PATCH v2] docs/misra: add 14.3

[Stefano Stabellini]

On Fri, 8 Sep 2023, Nicola Vetrini wrote:
> On 07/09/2023 23:45, Stefano Stabellini wrote:
> > On Thu, 7 Sep 2023, Jan Beulich wrote:
> > > On 07.09.2023 03:22, Stefano Stabellini wrote:
> > > > @@ -385,6 +386,17 @@ maintainers if you want to suggest a change.
> > > >       - A loop counter shall not have essentially floating type
> > > >       -
> > > >
> > > > +   * - `Rule 14.3
> > > <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_03.c>`_
> > > > +     - Required
> > > > +     - Controlling expressions shall not be invariant
> > > > +     - Due to the extensive usage of IS_ENABLED, sizeof compile-time
> > > > +       checks, and other constructs that are detected as errors by
> > > MISRA
> > > > +       C scanners, managing the configuration of a MISRA C scanner for
> > > > +       this rule would be unmanageable. Thus, this rule is adopted with
> > > > +       a project-wide deviation on if ?: and switch statements.
> > > 
> > > Do we want to go as far as permitting this uniformly for all switch()? In
> > > my earlier reply I had included sizeof() for a reason.
> > 
> > I agree with you that it would be better to restrict it to only some
> > switch uses, rather than all of them.
> > 
> > But if we are going to restrict the deviation to switch(sizeof()), which
> > I think is a good idea and I am in favor, wouldn't it be better to
> > handle these cases as individual deviations? E.g. docs/misra/safe.json?
> > I am assuming there are only few cases like that and adding it here
> > makes the rule more complicated.
> > 
> > I am happy either way but I wanted to provide that as an option.
> > 
> > 
> 
> It's also worth considering the magnitude of required SAF comments when making
> this choice.
> In this case, from a cursory glance at the present violations, it seems that
> there are maybe a
> handful of switches inside macros, so they shouldn't be problematic to deal
> with.

That's what I thought as well and why I suggested to only deal with them
in safe.json. However, it doesn't cost us anything to be extra clear, so
I'll send an updated patch with extra clarity.