Re: [PATCH] misra: add R14.4 R21.1 R21.2

[Jan Beulich <jbeulich@xxxxxxxx>]

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