docs/misra: add 14.3 and 14.4

[PATCH] docs/misra: add 14.3 and 14.4

Posted by Stefano Stabellini 8 months, 1 week ago

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

Add 14.3, with a project-wide deviations on if statements.
Add 14.4, clarifying that implicit conversions of integers, chars and
pointers to bool are allowed.

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>
---
 docs/misra/rules.rst | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index db30632b93..6cde4feeae 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -234,7 +234,7 @@ 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
-     -
+     - Function pointer types shall have named parameters too.
 
    * - `Rule 8.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_03.c>`_
      - Required
@@ -332,6 +332,24 @@ 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' statements. The rule only
+       applies to while, for, do ... while, ?:, and switch statements.
+
+   * - `Rule 14.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_04.c>`_
+     - Required
+     - The controlling expression of an if statement and the controlling
+       expression of an iteration-statement shall have essentially
+       Boolean type
+     - Implicit conversions of integers, pointers, and chars to boolean
+       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] docs/misra: add 14.3 and 14.4

Posted by Bertrand Marquis 8 months, 1 week ago

Hi Stefano,

> On 30 Aug 2023, at 02:59, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> From: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> Add 14.3, with a project-wide deviations on if statements.
> Add 14.4, clarifying that implicit conversions of integers, chars and
> pointers to bool are allowed.
> 
> 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>
> ---
> docs/misra/rules.rst | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> index db30632b93..6cde4feeae 100644
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -234,7 +234,7 @@ 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
> -     -
> +     - Function pointer types shall have named parameters too.


I would just modify to Function and Function pointers types shall be ...

> 
>    * - `Rule 8.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_03.c>`_
>      - Required
> @@ -332,6 +332,24 @@ 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' statements. The rule only
> +       applies to while, for, do ... while, ?:, and switch statements.

Didn't we also said that we would accept while(0) and while(1) ?
Also i agree with Jan, ? is really the same as if so we should not treat it differently.

> +
> +   * - `Rule 14.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_04.c>`_
> +     - Required
> +     - The controlling expression of an if statement and the controlling
> +       expression of an iteration-statement shall have essentially
> +       Boolean type
> +     - Implicit conversions of integers, pointers, and chars to boolean
> +       are allowed

I am a bit wondering here what is remaining after this deviation.

> +
>    * - `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
> 

Cheers
Bertrand

Re: [PATCH] docs/misra: add 14.3 and 14.4

Posted by Stefano Stabellini 8 months, 1 week ago

On Wed, 30 Aug 2023, Bertrand Marquis wrote:
> Hi Stefano,
> 
> > On 30 Aug 2023, at 02:59, Stefano Stabellini <sstabellini@kernel.org> wrote:
> > 
> > From: Stefano Stabellini <stefano.stabellini@amd.com>
> > 
> > Add 14.3, with a project-wide deviations on if statements.
> > Add 14.4, clarifying that implicit conversions of integers, chars and
> > pointers to bool are allowed.
> > 
> > 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>
> > ---
> > docs/misra/rules.rst | 20 +++++++++++++++++++-
> > 1 file changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> > index db30632b93..6cde4feeae 100644
> > --- a/docs/misra/rules.rst
> > +++ b/docs/misra/rules.rst
> > @@ -234,7 +234,7 @@ 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
> > -     -
> > +     - Function pointer types shall have named parameters too.
> 
> 
> I would just modify to Function and Function pointers types shall be ...

Sure, I can do that.


> > 
> >    * - `Rule 8.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_03.c>`_
> >      - Required
> > @@ -332,6 +332,24 @@ 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' statements. The rule only
> > +       applies to while, for, do ... while, ?:, and switch statements.
> 
> Didn't we also said that we would accept while(0) and while(1) ?
> Also i agree with Jan, ? is really the same as if so we should not treat it differently.

I took the list of things the rule applies to from the text of the rule
itself. However, I think you are right about the ?: and it should be
deviated together with if. I can also add while(0) and while(1).

Re: [PATCH] docs/misra: add 14.3 and 14.4

Posted by Jan Beulich 8 months, 1 week ago

On 30.08.2023 09:54, Bertrand Marquis wrote:
>> On 30 Aug 2023, at 02:59, Stefano Stabellini <sstabellini@kernel.org> wrote:
>> +   * - `Rule 14.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_04.c>`_
>> +     - Required
>> +     - The controlling expression of an if statement and the controlling
>> +       expression of an iteration-statement shall have essentially
>> +       Boolean type
>> +     - Implicit conversions of integers, pointers, and chars to boolean
>> +       are allowed
> 
> I am a bit wondering here what is remaining after this deviation.

Hmm, good point - floating point (and alike) types, which we don't use anyway.

Jan

Re: [PATCH] docs/misra: add 14.3 and 14.4

Posted by Bertrand Marquis 8 months, 1 week ago

Hi,

> On 30 Aug 2023, at 09:58, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 30.08.2023 09:54, Bertrand Marquis wrote:
>>> On 30 Aug 2023, at 02:59, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>> +   * - `Rule 14.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_04.c>`_
>>> +     - Required
>>> +     - The controlling expression of an if statement and the controlling
>>> +       expression of an iteration-statement shall have essentially
>>> +       Boolean type
>>> +     - Implicit conversions of integers, pointers, and chars to boolean
>>> +       are allowed
>> 
>> I am a bit wondering here what is remaining after this deviation.
> 
> Hmm, good point - floating point (and alike) types, which we don't use anyway.

So we accept the rule but we deviate all cases that would apply.
I do not think we should do that.

Bertrand

> 
> Jan
>

Re: [PATCH] docs/misra: add 14.3 and 14.4

Posted by Stefano Stabellini 8 months, 1 week ago

On Wed, 30 Aug 2023, Bertrand Marquis wrote:
> > On 30 Aug 2023, at 09:58, Jan Beulich <jbeulich@suse.com> wrote:
> > 
> > On 30.08.2023 09:54, Bertrand Marquis wrote:
> >>> On 30 Aug 2023, at 02:59, Stefano Stabellini <sstabellini@kernel.org> wrote:
> >>> +   * - `Rule 14.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_04.c>`_
> >>> +     - Required
> >>> +     - The controlling expression of an if statement and the controlling
> >>> +       expression of an iteration-statement shall have essentially
> >>> +       Boolean type
> >>> +     - Implicit conversions of integers, pointers, and chars to boolean
> >>> +       are allowed
> >> 
> >> I am a bit wondering here what is remaining after this deviation.
> > 
> > Hmm, good point - floating point (and alike) types, which we don't use anyway.
> 
> So we accept the rule but we deviate all cases that would apply.
> I do not think we should do that.

In the past we have been accepting rules that don't apply to Xen because
it is not much that they don't apply. It is that we have no violations
and we don't think there is any risk of getting violations in the
future.

But theoretically they apply, and for example if we end up with floating
points in Xen at some point in the future we would want rules affecting
floating points to apply, hence we accepted rules about floating points.

This rule is a bit different though: we deviate most of the rule and the
remaining part is small. In addition, we have no violations and we don't
think there is any risk of getting violations in the future for what's
left of the rule.

So I don't know if it is worth adding the rule or not. I think we should
ask Roberto next time. For now, I remove it from this patch.

Re: [PATCH] docs/misra: add 14.3 and 14.4

Posted by Jan Beulich 8 months, 1 week ago

On 30.08.2023 10:00, Bertrand Marquis wrote:
> Hi,
> 
>> On 30 Aug 2023, at 09:58, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 30.08.2023 09:54, Bertrand Marquis wrote:
>>>> On 30 Aug 2023, at 02:59, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>>> +   * - `Rule 14.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_04.c>`_
>>>> +     - Required
>>>> +     - The controlling expression of an if statement and the controlling
>>>> +       expression of an iteration-statement shall have essentially
>>>> +       Boolean type
>>>> +     - Implicit conversions of integers, pointers, and chars to boolean
>>>> +       are allowed
>>>
>>> I am a bit wondering here what is remaining after this deviation.
>>
>> Hmm, good point - floating point (and alike) types, which we don't use anyway.
> 
> So we accept the rule but we deviate all cases that would apply.
> I do not think we should do that.

Right; Stefano - I withdraw my offer to ack a 14.4-only patch.

Jan

Re: [PATCH] docs/misra: add 14.3 and 14.4

Posted by Jan Beulich 8 months, 1 week ago

On 30.08.2023 02:59, Stefano Stabellini wrote:
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -234,7 +234,7 @@ 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
> -     -
> +     - Function pointer types shall have named parameters too.

This isn't an exception; do we really need to state such? I would have
expected something to appear here only if we intended to deviate certain
constructs.

> @@ -332,6 +332,24 @@ 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' statements. The rule only
> +       applies to while, for, do ... while, ?:, and switch statements.

The sizeof() aspect mentioned particularly applies to switch() as well.
Furthermore ?: is really only shorthand for simple if(), so I don't see
treating it different from if() as helpful.

That said, I'd be a little hesitant to give an ack here anyway. If you'd
split 14.3 and 14.4, I'd be happy to ack 14.4's addition.

Jan

> +   * - `Rule 14.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_04.c>`_
> +     - Required
> +     - The controlling expression of an if statement and the controlling
> +       expression of an iteration-statement shall have essentially
> +       Boolean type
> +     - Implicit conversions of integers, pointers, and chars to boolean
> +       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

Re: [PATCH] docs/misra: add 14.3 and 14.4

Posted by Stefano Stabellini 8 months, 1 week ago

On Wed, 30 Aug 2023, Jan Beulich wrote:
> On 30.08.2023 02:59, Stefano Stabellini wrote:
> > --- a/docs/misra/rules.rst
> > +++ b/docs/misra/rules.rst
> > @@ -234,7 +234,7 @@ 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
> > -     -
> > +     - Function pointer types shall have named parameters too.
> 
> This isn't an exception; do we really need to state such? I would have
> expected something to appear here only if we intended to deviate certain
> constructs.

Yes, it is not an exception. However, as there was genuine confusion in
the community about whether the rule should apply or not to function
pointer types I think it would be good to clarify. To avoid any doubts
in the future. My preference is to keep this as clarification.



> > @@ -332,6 +332,24 @@ 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' statements. The rule only
> > +       applies to while, for, do ... while, ?:, and switch statements.
> 
> The sizeof() aspect mentioned particularly applies to switch() as well.
> Furthermore ?: is really only shorthand for simple if(), so I don't see
> treating it different from if() as helpful.

I'll answer in another email.



> That said, I'd be a little hesitant to give an ack here anyway. If you'd
> split 14.3 and 14.4, I'd be happy to ack 14.4's addition.
> 
> Jan
> 
> > +   * - `Rule 14.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_04.c>`_
> > +     - Required
> > +     - The controlling expression of an if statement and the controlling
> > +       expression of an iteration-statement shall have essentially
> > +       Boolean type
> > +     - Implicit conversions of integers, pointers, and chars to boolean
> > +       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
>