Update docs/misra/rules.rst to reflect the MISRA C rules accepted in the
last couple of months.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---

In the notes section I added some info about the deviations, but in any
case the appropriate info will also be added to deviations.rst,
safe.json, etc.

I also added Rule 14.4, which is older, but when I first tried to add it
to rules.rst, Jan had a question I couldn't reply clearly:
https://marc.info/?l=xen-devel&m=169828285627163

I think now with this series, the impact of Rule 14.4 is clearer:
https://marc.info/?l=xen-devel&m=170194257326186

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 182331089d..0cb87ebc35 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -181,6 +181,12 @@ maintainers if you want to suggest a change.
        headers (xen/include/public/) are allowed to retain longer
        identifiers for backward compatibility.
 
+   * - `Rule 5.5 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_05_05.c>`_
+     - Required
+     - Identifiers shall be distinct from macro names
+     - Clashes between function-like macros and non-callable entities
+       are allowed. The pattern #define x x is also allowed.
+
    * - `Rule 5.6 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_05_06.c>`_
      - Required
      - A typedef name shall be a unique identifier
@@ -462,6 +468,13 @@ maintainers if you want to suggest a change.
 
        while(0) and while(1) and alike are allowed.
 
+   * - `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
+     -
+
    * - `Rule 16.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_16_03.c>`_
      - Required
      - An unconditional break statement shall terminate every
@@ -478,6 +491,12 @@ maintainers if you want to suggest a change.
        adhere to Rule 16.2 would result in increased complexity and
        maintenance difficulty, and could potentially introduce bugs. 
 
+   * - `Rule 16.6 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_16_06.c>`_
+     - Required
+     - Every switch statement shall have at least two switch-clauses
+     - Single-clause switches are allowed when they do not involve a
+       default label.
+
    * - `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
@@ -554,6 +573,13 @@ maintainers if you want to suggest a change.
        evaluation
      -
 
+   * - `Rule 20.12 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_20_12.c>`_
+     - A macro parameter used as an operand to the # or ## operators,
+       which is itself subject to further macro replacement, shall only
+       be used as an operand to these operators
+     - Required
+     - Variadic macros are allowed to violate the rule.
+
    * - `Rule 20.13 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_20_13.c>`_
      - Required
      - A line whose first token is # shall be a valid preprocessing

On 10.02.2024 02:00, Stefano Stabellini wrote:
> Update docs/misra/rules.rst to reflect the MISRA C rules accepted in the
> last couple of months.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
> 
> In the notes section I added some info about the deviations, but in any
> case the appropriate info will also be added to deviations.rst,
> safe.json, etc.
> 
> I also added Rule 14.4, which is older, but when I first tried to add it
> to rules.rst, Jan had a question I couldn't reply clearly:
> https://marc.info/?l=xen-devel&m=169828285627163
> 
> I think now with this series, the impact of Rule 14.4 is clearer:
> https://marc.info/?l=xen-devel&m=170194257326186

This series is about enums only afaics. Yet the rule is much wider, and iirc
we had agreed that for integer and pointer types the normal language
conversion to boolean meaning is fine as well. Not only do you not mention
this case in the entry, but it also continue to mean that effectively we
limit the rule to a very narrow case. Which continue to leave open the
question of whether the rule is worthwhile to accept in the first place.

Jan

On Mon, 12 Feb 2024, Jan Beulich wrote:
> On 10.02.2024 02:00, Stefano Stabellini wrote:
> > Update docs/misra/rules.rst to reflect the MISRA C rules accepted in the
> > last couple of months.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > ---
> > 
> > In the notes section I added some info about the deviations, but in any
> > case the appropriate info will also be added to deviations.rst,
> > safe.json, etc.
> > 
> > I also added Rule 14.4, which is older, but when I first tried to add it
> > to rules.rst, Jan had a question I couldn't reply clearly:
> > https://marc.info/?l=xen-devel&m=169828285627163
> > 
> > I think now with this series, the impact of Rule 14.4 is clearer:
> > https://marc.info/?l=xen-devel&m=170194257326186
> 
> This series is about enums only afaics. Yet the rule is much wider, and iirc
> we had agreed that for integer and pointer types the normal language
> conversion to boolean meaning is fine as well. Not only do you not mention
> this case in the entry,

I can add a note about it.


> but it also continue to mean that effectively we
> limit the rule to a very narrow case. Which continue to leave open the
> question of whether the rule is worthwhile to accept in the first place.

When someone does a safety certification, there is a difference between
deviating a rule as a whole or accepting the rule and only deviating
certain aspects of it (simply ignoring the rule is typically not an
option in safety certification context.) So here I think it would help
downstreams interested in safety if we added the rule, with specific
deviations.

Do you have any comments on the other parts of this patch? If not, I
would be happy to resent the rest unmodified, and update only 14.4 in
its own separate patch where we can discuss further.

On 13.02.2024 00:18, Stefano Stabellini wrote:
> On Mon, 12 Feb 2024, Jan Beulich wrote:
>> On 10.02.2024 02:00, Stefano Stabellini wrote:
>>> Update docs/misra/rules.rst to reflect the MISRA C rules accepted in the
>>> last couple of months.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>> ---
>>>
>>> In the notes section I added some info about the deviations, but in any
>>> case the appropriate info will also be added to deviations.rst,
>>> safe.json, etc.
>>>
>>> I also added Rule 14.4, which is older, but when I first tried to add it
>>> to rules.rst, Jan had a question I couldn't reply clearly:
>>> https://marc.info/?l=xen-devel&m=169828285627163
>>>
>>> I think now with this series, the impact of Rule 14.4 is clearer:
>>> https://marc.info/?l=xen-devel&m=170194257326186
>>
>> This series is about enums only afaics. Yet the rule is much wider, and iirc
>> we had agreed that for integer and pointer types the normal language
>> conversion to boolean meaning is fine as well. Not only do you not mention
>> this case in the entry,
> 
> I can add a note about it.
> 
> 
>> but it also continue to mean that effectively we
>> limit the rule to a very narrow case. Which continue to leave open the
>> question of whether the rule is worthwhile to accept in the first place.
> 
> When someone does a safety certification, there is a difference between
> deviating a rule as a whole or accepting the rule and only deviating
> certain aspects of it (simply ignoring the rule is typically not an
> option in safety certification context.) So here I think it would help
> downstreams interested in safety if we added the rule, with specific
> deviations.

Yet then in other cases you refer to Bertrand's general statement of it
not being helpful when too little of a rule is left by deviating.

> Do you have any comments on the other parts of this patch? If not, I
> would be happy to resent the rest unmodified, and update only 14.4 in
> its own separate patch where we can discuss further.

Well. We're in territory now where I'm not really happy anymore with the
full scope of what is being added to the "accepted" list. Leaving 14.4
aside, what you have in the patch all looks like what was agreed upon,
but then I'm not taking notes during meetings, and hence I can't help
the impression that e.g. for 5.5 there was more than just the one
"permitted" pattern. Therefore, while I deliberately didn't comment
there (for not having a concrete case in mind), I'm afraid I also don't
feel anymore like acking such multi-rule patches. If you strictly went
one by one, it is certainly possible that I might ack this and that.

As attempted to voice several times during the meetings, I pretty
strongly disagree with many of the "developer confusion" aspects, when
they take away options the language quite obviously and naturally
provides. We're talking about hypervisor code here, not some random
tool that was thrown together in a haste. At the risk of sounding
arrogant, people being easily confused by what I'd call normal code
should simply not touch code like this. Whereas the spirit of many of
these rules looks to rather go in the direction that basically anyone
knowing a little bit of C should be qualified enough to maintain code
made subject to all of these rules.

I'm sorry, Jan

On Tue, 13 Feb 2024, Jan Beulich wrote:
> On 13.02.2024 00:18, Stefano Stabellini wrote:
> > On Mon, 12 Feb 2024, Jan Beulich wrote:
> >> On 10.02.2024 02:00, Stefano Stabellini wrote:
> >>> Update docs/misra/rules.rst to reflect the MISRA C rules accepted in the
> >>> last couple of months.
> >>>
> >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> >>> ---
> >>>
> >>> In the notes section I added some info about the deviations, but in any
> >>> case the appropriate info will also be added to deviations.rst,
> >>> safe.json, etc.
> >>>
> >>> I also added Rule 14.4, which is older, but when I first tried to add it
> >>> to rules.rst, Jan had a question I couldn't reply clearly:
> >>> https://marc.info/?l=xen-devel&m=169828285627163
> >>>
> >>> I think now with this series, the impact of Rule 14.4 is clearer:
> >>> https://marc.info/?l=xen-devel&m=170194257326186
> >>
> >> This series is about enums only afaics. Yet the rule is much wider, and iirc
> >> we had agreed that for integer and pointer types the normal language
> >> conversion to boolean meaning is fine as well. Not only do you not mention
> >> this case in the entry,
> > 
> > I can add a note about it.
> > 
> > 
> >> but it also continue to mean that effectively we
> >> limit the rule to a very narrow case. Which continue to leave open the
> >> question of whether the rule is worthwhile to accept in the first place.
> > 
> > When someone does a safety certification, there is a difference between
> > deviating a rule as a whole or accepting the rule and only deviating
> > certain aspects of it (simply ignoring the rule is typically not an
> > option in safety certification context.) So here I think it would help
> > downstreams interested in safety if we added the rule, with specific
> > deviations.
> 
> Yet then in other cases you refer to Bertrand's general statement of it
> not being helpful when too little of a rule is left by deviating.

I think it is a case-by-case judgement call. Also, keep in mind that
Bertrand's goal was to encourage us to accept more rules in their
entirety without any deviations, not reject more rules. Especially as
rejecting rules it is typically not possible for people doing safety, so
the alternative is to write a wider deviation -- deviate even more, not
less.


> > Do you have any comments on the other parts of this patch? If not, I
> > would be happy to resent the rest unmodified, and update only 14.4 in
> > its own separate patch where we can discuss further.
> 
> Well. We're in territory now where I'm not really happy anymore with the
> full scope of what is being added to the "accepted" list. Leaving 14.4
> aside, what you have in the patch all looks like what was agreed upon,
> but then I'm not taking notes during meetings, and hence I can't help
> the impression that e.g. for 5.5 there was more than just the one
> "permitted" pattern. Therefore, while I deliberately didn't comment
> there (for not having a concrete case in mind), I'm afraid I also don't
> feel anymore like acking such multi-rule patches. If you strictly went
> one by one, it is certainly possible that I might ack this and that.

That's understandable especially as this time I was slower to send out
the patch to update docs/misra/rules.rst (apologies for that.)

Specifically for 5.5, I have the following notes from Roberto:
"""
We tag as deliberate all violations involving clashes
between function-like macros and non-callable entities.
We also tag as deliberate the violations matching the pattern #define x x.
In the future, once string.h will be restricted to the <string.h> functionality
of the C standard, string.h will be deviated further.
Further deviations are yet to be agreed upon (e.g., on ARM64 the deviation
of TYPE_SAFE resolves most of the violation; on x86_64 the deviation
of TYPE_SAFE resolves half of the violations but the others depend
on only about a dozen of macros).
"""

I added to the notes section only the part that I thought would make
sense to keep there. For the rest, I think it would be best to leave it
to future updates of documenting-violations.rst and/or safe.json as
appropriate.

I'll split out 14.4 and 5.5, and keep the rest in one patch.


> As attempted to voice several times during the meetings, I pretty
> strongly disagree with many of the "developer confusion" aspects, when
> they take away options the language quite obviously and naturally
> provides. We're talking about hypervisor code here, not some random
> tool that was thrown together in a haste. At the risk of sounding
> arrogant, people being easily confused by what I'd call normal code
> should simply not touch code like this. Whereas the spirit of many of
> these rules looks to rather go in the direction that basically anyone
> knowing a little bit of C should be qualified enough to maintain code
> made subject to all of these rules.

At some point there will be new people working on Xen (among others,
AMD has been hiring). New people need to start from somewhere. So the
less confusing the code the better it is because it is also less error
prone. There is code reviews and tests, but if the code was less prone
to misinterpretations, then there would be fewer chances of errors in
the first place.