Add 14.4, with the same note and exception already listed for 10.1.

Add 21.1 and 21.2, with a longer comment to explain how strategy with
leading underscores and why we think we are safe today.

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

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index b423580b23..56eec8bafd 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -422,6 +422,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
+     - Implicit conversions 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
@@ -479,6 +486,24 @@ maintainers if you want to suggest a change.
        they are related
      -
 
+   * - `Rule 21.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_21_01.c>`_
+     - Required
+     - #define and #undef shall not be used on a reserved identifier or
+       reserved macro name
+     - No identifiers should start with _BUILTIN to avoid clashes with
+       GCC reserved identifiers. In general, all identifiers starting with
+       an underscore are reserved, and are best avoided. However, Xen
+       makes extensive usage of leading underscores in header guards,
+       bitwise manipulation functions, and a few other places. They are
+       considered safe as checks have been done against the list of
+       GCC's reserved identifiers. They don't need to be replaced.
+
+   * - `Rule 21.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_21_02.c>`_
+     - Required
+     - A reserved identifier or reserved macro name shall not be
+       declared
+     - See comment for Rule 21.1
+
    * - `Rule 21.13 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_21_13.c>`_
      - Mandatory
      - Any value passed to a function in <ctype.h> shall be representable as an

On 24.10.2023 01:31, Stefano Stabellini wrote:> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -422,6 +422,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
> +     - Implicit conversions to boolean are allowed

What, if anything, remains of this rule with this exception?

> @@ -479,6 +486,24 @@ maintainers if you want to suggest a change.
>         they are related
>       -
>  
> +   * - `Rule 21.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_21_01.c>`_
> +     - Required
> +     - #define and #undef shall not be used on a reserved identifier or
> +       reserved macro name
> +     - No identifiers should start with _BUILTIN to avoid clashes with

DYM "__builtin_"? Also gcc introduces far more than just __builtin_...()
into the name space.

> +       GCC reserved identifiers. In general, all identifiers starting with
> +       an underscore are reserved, and are best avoided.

This isn't precise enough. A leading underscore followed by a lower-case
letter or a number is okay to use for file-scope symbols. Imo we should
not aim at removing such uses, but rather encourage more use.

In this context, re-reading some of the C99 spec, I have to realize that
my commenting on underscore-prefixed macro parameters (but not underscore-
prefixed locals in macros) was based on ambiguous information in the spec.
I will try to remember to not comment on such anymore going forward.

> However, Xen
> +       makes extensive usage of leading underscores in header guards,
> +       bitwise manipulation functions, and a few other places. They are
> +       considered safe as checks have been done against the list of
> +       GCC's reserved identifiers. They don't need to be replaced.

This leaves quite vague what wants and what does not want replacing, nor
what might be okay to introduce subsequently despite violation this rule.

Jan

On Tue, 24 Oct 2023, Jan Beulich wrote:
> On 24.10.2023 01:31, Stefano Stabellini wrote:> --- a/docs/misra/rules.rst
> > +++ b/docs/misra/rules.rst
> > @@ -422,6 +422,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
> > +     - Implicit conversions to boolean are allowed
> 
> What, if anything, remains of this rule with this exception?

Not much, but there is a difference between following a rule with a
deviation (in this case we allow implicit conversions of integers and
pointers to bool because we claim all Xen developers know how they work)
and not follow the rule at all, at least for the assessors. Also,
anything that doesn't implicitly convert to a boolean is not allowed,
although I guess probably it wouldn't compile either.

We could also try to find a better wording to reduce the deviation only
to integer and pointers. Any suggestions?


> > @@ -479,6 +486,24 @@ maintainers if you want to suggest a change.
> >         they are related
> >       -
> >  
> > +   * - `Rule 21.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_21_01.c>`_
> > +     - Required
> > +     - #define and #undef shall not be used on a reserved identifier or
> > +       reserved macro name
> > +     - No identifiers should start with _BUILTIN to avoid clashes with
> 
> DYM "__builtin_"? Also gcc introduces far more than just __builtin_...()
> into the name space.

Yes agreed, but in my notes that is the only one I wrote down. I recall
that the complete list is a couple of pages long, I don't think we can
possibly add it here in full and if I recall it is not available in the
GCC documentation. More on this below.


> > +       GCC reserved identifiers. In general, all identifiers starting with
> > +       an underscore are reserved, and are best avoided.
> 
> This isn't precise enough. A leading underscore followed by a lower-case
> letter or a number is okay to use for file-scope symbols. Imo we should
> not aim at removing such uses, but rather encourage more use.

More on this below


> In this context, re-reading some of the C99 spec, I have to realize that
> my commenting on underscore-prefixed macro parameters (but not underscore-
> prefixed locals in macros) was based on ambiguous information in the spec.
> I will try to remember to not comment on such anymore going forward.
>
> > However, Xen
> > +       makes extensive usage of leading underscores in header guards,
> > +       bitwise manipulation functions, and a few other places. They are
> > +       considered safe as checks have been done against the list of
> > +       GCC's reserved identifiers. They don't need to be replaced.
> 
> This leaves quite vague what wants and what does not want replacing, nor
> what might be okay to introduce subsequently despite violation this rule.
 
My goals here were to convey the following:
1) avoid clashes with gcc reserved identifiers
2) in general try to reduce the usage of leading underscores except for
   known existing locations (header guards, bitwise manipulation
   functions)

However, for 1) the problem is that we don't have the full list and for
2) the problem is that it is too vague.

Instead I suggest we do the following:
- we get the full list of gcc reserved identifiers from Roberto and we
  commit it to xen.git as a seprate file under docs/misra
- we reference the list here saying one should avoid clashes with those
  identifiers as reserved by gcc


And if we can find a clear general comment about the usage of leading
underscores in Xen I am happy to add it too (e.g. header guards), but
from MISRA point of view the above is sufficient.

On 25.10.2023 03:15, Stefano Stabellini wrote:
> On Tue, 24 Oct 2023, Jan Beulich wrote:
>> On 24.10.2023 01:31, Stefano Stabellini wrote:> --- a/docs/misra/rules.rst
>>> +++ b/docs/misra/rules.rst
>>> @@ -422,6 +422,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
>>> +     - Implicit conversions to boolean are allowed
>>
>> What, if anything, remains of this rule with this exception?
> 
> Not much, but there is a difference between following a rule with a
> deviation (in this case we allow implicit conversions of integers and
> pointers to bool because we claim all Xen developers know how they work)
> and not follow the rule at all, at least for the assessors. Also,
> anything that doesn't implicitly convert to a boolean is not allowed,
> although I guess probably it wouldn't compile either.
> 
> We could also try to find a better wording to reduce the deviation only
> to integer and pointers. Any suggestions?

Since compound types can't be converted anyway, I think only floating
point types (and their relatives) remain, which we don't use anyway
(and if we did, excepting them as well would only be logical imo). I
therefore see little point in making "integers and pointers" explicit.

Instead I wonder if we shouldn't make ourselves honest and say we
deviate this rule as a whole.

>>> @@ -479,6 +486,24 @@ maintainers if you want to suggest a change.
>>>         they are related
>>>       -
>>>  
>>> +   * - `Rule 21.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_21_01.c>`_
>>> +     - Required
>>> +     - #define and #undef shall not be used on a reserved identifier or
>>> +       reserved macro name
>>> +     - No identifiers should start with _BUILTIN to avoid clashes with
>>
>> DYM "__builtin_"? Also gcc introduces far more than just __builtin_...()
>> into the name space.
> 
> Yes agreed, but in my notes that is the only one I wrote down. I recall
> that the complete list is a couple of pages long, I don't think we can
> possibly add it here in full and if I recall it is not available in the
> GCC documentation. More on this below.
> 
> 
>>> +       GCC reserved identifiers. In general, all identifiers starting with
>>> +       an underscore are reserved, and are best avoided.
>>
>> This isn't precise enough. A leading underscore followed by a lower-case
>> letter or a number is okay to use for file-scope symbols. Imo we should
>> not aim at removing such uses, but rather encourage more use.
> 
> More on this below
> 
> 
>> In this context, re-reading some of the C99 spec, I have to realize that
>> my commenting on underscore-prefixed macro parameters (but not underscore-
>> prefixed locals in macros) was based on ambiguous information in the spec.
>> I will try to remember to not comment on such anymore going forward.
>>
>>> However, Xen
>>> +       makes extensive usage of leading underscores in header guards,
>>> +       bitwise manipulation functions, and a few other places. They are
>>> +       considered safe as checks have been done against the list of
>>> +       GCC's reserved identifiers. They don't need to be replaced.
>>
>> This leaves quite vague what wants and what does not want replacing, nor
>> what might be okay to introduce subsequently despite violation this rule.
>  
> My goals here were to convey the following:
> 1) avoid clashes with gcc reserved identifiers
> 2) in general try to reduce the usage of leading underscores except for
>    known existing locations (header guards, bitwise manipulation
>    functions)
> 
> However, for 1) the problem is that we don't have the full list and for
> 2) the problem is that it is too vague.
> 
> Instead I suggest we do the following:
> - we get the full list of gcc reserved identifiers from Roberto and we
>   commit it to xen.git as a seprate file under docs/misra

Such a full list can only be compiled for any specific gcc version. Even
non-upstream backports by a distro may alter this list.

> - we reference the list here saying one should avoid clashes with those
>   identifiers as reserved by gcc

With the list constantly changing (mostly expanding), that's pretty
hopeless.

> And if we can find a clear general comment about the usage of leading
> underscores in Xen I am happy to add it too (e.g. header guards), but
> from MISRA point of view the above is sufficient.

But what we need isn't a description of the status quo, but one of
what state we want to (slowly) move to. The status quo anyway is
"no pattern, as in the past hardly anyone cared".

Jan

On Wed, 25 Oct 2023, Jan Beulich wrote:
> On 25.10.2023 03:15, Stefano Stabellini wrote:
> > On Tue, 24 Oct 2023, Jan Beulich wrote:
> >> On 24.10.2023 01:31, Stefano Stabellini wrote:> --- a/docs/misra/rules.rst
> >>> +++ b/docs/misra/rules.rst
> >>> @@ -422,6 +422,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
> >>> +     - Implicit conversions to boolean are allowed
> >>
> >> What, if anything, remains of this rule with this exception?
> > 
> > Not much, but there is a difference between following a rule with a
> > deviation (in this case we allow implicit conversions of integers and
> > pointers to bool because we claim all Xen developers know how they work)
> > and not follow the rule at all, at least for the assessors. Also,
> > anything that doesn't implicitly convert to a boolean is not allowed,
> > although I guess probably it wouldn't compile either.
> > 
> > We could also try to find a better wording to reduce the deviation only
> > to integer and pointers. Any suggestions?
> 
> Since compound types can't be converted anyway, I think only floating
> point types (and their relatives) remain, which we don't use anyway
> (and if we did, excepting them as well would only be logical imo). I
> therefore see little point in making "integers and pointers" explicit.
> 
> Instead I wonder if we shouldn't make ourselves honest and say we
> deviate this rule as a whole.

Yes, I see your point. I think I'll remove Rule 14.4 from the patch and
put it in the bucket of all the rules deviated as a whole (which we
should track as a separate rst file but today we don't.)


> >>> @@ -479,6 +486,24 @@ maintainers if you want to suggest a change.
> >>>         they are related
> >>>       -
> >>>  
> >>> +   * - `Rule 21.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_21_01.c>`_
> >>> +     - Required
> >>> +     - #define and #undef shall not be used on a reserved identifier or
> >>> +       reserved macro name
> >>> +     - No identifiers should start with _BUILTIN to avoid clashes with
> >>
> >> DYM "__builtin_"? Also gcc introduces far more than just __builtin_...()
> >> into the name space.
> > 
> > Yes agreed, but in my notes that is the only one I wrote down. I recall
> > that the complete list is a couple of pages long, I don't think we can
> > possibly add it here in full and if I recall it is not available in the
> > GCC documentation. More on this below.
> > 
> > 
> >>> +       GCC reserved identifiers. In general, all identifiers starting with
> >>> +       an underscore are reserved, and are best avoided.
> >>
> >> This isn't precise enough. A leading underscore followed by a lower-case
> >> letter or a number is okay to use for file-scope symbols. Imo we should
> >> not aim at removing such uses, but rather encourage more use.
> > 
> > More on this below
> > 
> > 
> >> In this context, re-reading some of the C99 spec, I have to realize that
> >> my commenting on underscore-prefixed macro parameters (but not underscore-
> >> prefixed locals in macros) was based on ambiguous information in the spec.
> >> I will try to remember to not comment on such anymore going forward.
> >>
> >>> However, Xen
> >>> +       makes extensive usage of leading underscores in header guards,
> >>> +       bitwise manipulation functions, and a few other places. They are
> >>> +       considered safe as checks have been done against the list of
> >>> +       GCC's reserved identifiers. They don't need to be replaced.
> >>
> >> This leaves quite vague what wants and what does not want replacing, nor
> >> what might be okay to introduce subsequently despite violation this rule.
> >  
> > My goals here were to convey the following:
> > 1) avoid clashes with gcc reserved identifiers
> > 2) in general try to reduce the usage of leading underscores except for
> >    known existing locations (header guards, bitwise manipulation
> >    functions)
> > 
> > However, for 1) the problem is that we don't have the full list and for
> > 2) the problem is that it is too vague.
> > 
> > Instead I suggest we do the following:
> > - we get the full list of gcc reserved identifiers from Roberto and we
> >   commit it to xen.git as a seprate file under docs/misra
> 
> Such a full list can only be compiled for any specific gcc version. Even
> non-upstream backports by a distro may alter this list.
> 
> > - we reference the list here saying one should avoid clashes with those
> >   identifiers as reserved by gcc
> 
> With the list constantly changing (mostly expanding), that's pretty
> hopeless.
> 
> > And if we can find a clear general comment about the usage of leading
> > underscores in Xen I am happy to add it too (e.g. header guards), but
> > from MISRA point of view the above is sufficient.
> 
> But what we need isn't a description of the status quo, but one of
> what state we want to (slowly) move to. The status quo anyway is
> "no pattern, as in the past hardly anyone cared".

I guess you are suggesting something more like the following, but please
feel free to suggest your favorite wording (it might be easier for me to
understand what you mean if you provide a short example).

---
All identifiers starting with an underscore are reserved and should not
be used. Today Xen uses many, such as header guards and bitwise
manipulation functions. Upon analysis it turns out Xen identifiers do
not clash with the identifiers used by modern GCC, but that is not a
guarantee that there won't be a naming clash in the future or with
another compiler. For these reasons we discourage the introduction of
new reserved identifiers in Xen, and we see it as positive the reduction
of reserved identifiers. At the same time, certain identifiers starting
with an underscore are also commonly used in Linux (e.g. __set_bit) and
we don't think it would be an improvement to rename them.

On 26.10.2023 03:16, Stefano Stabellini wrote:
> On Wed, 25 Oct 2023, Jan Beulich wrote:
>> On 25.10.2023 03:15, Stefano Stabellini wrote:
>>> And if we can find a clear general comment about the usage of leading
>>> underscores in Xen I am happy to add it too (e.g. header guards), but
>>> from MISRA point of view the above is sufficient.
>>
>> But what we need isn't a description of the status quo, but one of
>> what state we want to (slowly) move to. The status quo anyway is
>> "no pattern, as in the past hardly anyone cared".
> 
> I guess you are suggesting something more like the following, but please
> feel free to suggest your favorite wording (it might be easier for me to
> understand what you mean if you provide a short example).
> 
> ---
> All identifiers starting with an underscore are reserved and should not
> be used.

Again, no. Identifiers starting with an underscore followed by another
underscore or an upper-case letter are reserved. Other identifiers are
dedicated for a particular purpose, and are fine to use for (at least)
that purpose.

> Today Xen uses many, such as header guards and bitwise
> manipulation functions. Upon analysis it turns out Xen identifiers do
> not clash with the identifiers used by modern GCC, but that is not a
> guarantee that there won't be a naming clash in the future or with
> another compiler. For these reasons we discourage the introduction of
> new reserved identifiers in Xen, and we see it as positive the reduction
> of reserved identifiers. At the same time, certain identifiers starting
> with an underscore are also commonly used in Linux (e.g. __set_bit) and
> we don't think it would be an improvement to rename them.

Everything else reads okay-ish to me.

Jan

On Thu, 26 Oct 2023, Jan Beulich wrote:
> On 26.10.2023 03:16, Stefano Stabellini wrote:
> > On Wed, 25 Oct 2023, Jan Beulich wrote:
> >> On 25.10.2023 03:15, Stefano Stabellini wrote:
> >>> And if we can find a clear general comment about the usage of leading
> >>> underscores in Xen I am happy to add it too (e.g. header guards), but
> >>> from MISRA point of view the above is sufficient.
> >>
> >> But what we need isn't a description of the status quo, but one of
> >> what state we want to (slowly) move to. The status quo anyway is
> >> "no pattern, as in the past hardly anyone cared".
> > 
> > I guess you are suggesting something more like the following, but please
> > feel free to suggest your favorite wording (it might be easier for me to
> > understand what you mean if you provide a short example).
> > 
> > ---
> > All identifiers starting with an underscore are reserved and should not
> > be used.
> 
> Again, no. Identifiers starting with an underscore followed by another
> underscore or an upper-case letter are reserved. Other identifiers are
> dedicated for a particular purpose, and are fine to use for (at least)
> that purpose.
> 
> > Today Xen uses many, such as header guards and bitwise
> > manipulation functions. Upon analysis it turns out Xen identifiers do
> > not clash with the identifiers used by modern GCC, but that is not a
> > guarantee that there won't be a naming clash in the future or with
> > another compiler. For these reasons we discourage the introduction of
> > new reserved identifiers in Xen, and we see it as positive the reduction
> > of reserved identifiers. At the same time, certain identifiers starting
> > with an underscore are also commonly used in Linux (e.g. __set_bit) and
> > we don't think it would be an improvement to rename them.
> 
> Everything else reads okay-ish to me.

OK, here is the new version

Identifiers starting with an underscore followed by another underscore
or an upper-case letter are reserved. Today Xen uses many, such as
header guards and bitwise manipulation functions. Upon analysis it turns
out Xen identifiers do not clash with the identifiers used by modern
GCC, but that is not a guarantee that there won't be a naming clash in
the future or with another compiler.  For these reasons we discourage
the introduction of new reserved identifiers in Xen, and we see it as
positive the reduction of reserved identifiers. At the same time,
certain identifiers starting with an underscore are also commonly used
in Linux (e.g. __set_bit) and we don't think it would be an improvement
to rename them.