| Message ID | 20230830005950.305085-1-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | docs/misra: add 14.3 and 14.4 | expand |
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
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
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
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 >
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
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 >
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).
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.
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