Re: [PATCH] docs/misra/rules.rst: add more rules

[Jan Beulich]

On 08.12.2023 01:08, Stefano Stabellini wrote:
> On Thu, 7 Dec 2023, Jan Beulich wrote:
>> On 07.12.2023 03:42, Stefano Stabellini wrote:
>>> On Wed, 6 Dec 2023, Jan Beulich wrote:
 On 06.12.2023 04:02, Stefano Stabellini wrote:
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -462,11 +462,23 @@ maintainers if you want to suggest a change.
>  
> while(0) and while(1) and alike are allowed.
>  
> +   * - `Rule 16.3 
> `_
> + - Required
> + - An unconditional break statement shall terminate every
> +   switch-clause
> + - In addition to break, also other flow control statements such as
> +   continue, return, goto are allowed.
> +
> * - `Rule 16.7 
> `_
>   - Required
>   - A switch-expression shall not have essentially Boolean type
>   -
>  
> +   * - `Rule 17.1 
> `_
> + - Required
> + - The features of  shall not be used
> + -

 Did we really accept this without any constraint (warranting mentioning
 here)?
>>>
>>> We agreed that in certain situations stdarg.h is OK to use and in those
>>> cases we would add a deviation. Would you like me to add something to
>>> that effect here? I could do that but it would sound a bit vague.  Also
>>> if we want to specify a project-wide deviation it would be better
>>> documented in docs/misra/deviations.rst. I would leave Rule 17.1 without
>>> a note.
>>
>> I can see your point, and I don't have a good suggestion on possible text.
>> Still I wouldn't feel well ack-ing this in its present shape.
> 
> What about:
> 
>  - It is understood that in some limited circumstances  is
>appropriate to use, such as the implementation of printk. Those
>cases will be dealt with using deviations as usual, see
>docs/misra/deviations.rst and
>docs/misra/documenting-violations.rst.

Looks okay. Would also look okay if it was just the first sentence.

Jan

Re: [PATCH] docs/misra/rules.rst: add more rules

[Stefano Stabellini]

On Thu, 7 Dec 2023, Jan Beulich wrote:
> On 07.12.2023 03:42, Stefano Stabellini wrote:
> > On Wed, 6 Dec 2023, Jan Beulich wrote:
> >> On 06.12.2023 04:02, Stefano Stabellini wrote:
> >>> --- a/docs/misra/rules.rst
> >>> +++ b/docs/misra/rules.rst
> >>> @@ -462,11 +462,23 @@ maintainers if you want to suggest a change.
> >>>  
> >>> while(0) and while(1) and alike are allowed.
> >>>  
> >>> +   * - `Rule 16.3 
> >>> `_
> >>> + - Required
> >>> + - An unconditional break statement shall terminate every
> >>> +   switch-clause
> >>> + - In addition to break, also other flow control statements such as
> >>> +   continue, return, goto are allowed.
> >>> +
> >>> * - `Rule 16.7 
> >>> `_
> >>>   - Required
> >>>   - A switch-expression shall not have essentially Boolean type
> >>>   -
> >>>  
> >>> +   * - `Rule 17.1 
> >>> `_
> >>> + - Required
> >>> + - The features of  shall not be used
> >>> + -
> >>
> >> Did we really accept this without any constraint (warranting mentioning
> >> here)?
> > 
> > We agreed that in certain situations stdarg.h is OK to use and in those
> > cases we would add a deviation. Would you like me to add something to
> > that effect here? I could do that but it would sound a bit vague.  Also
> > if we want to specify a project-wide deviation it would be better
> > documented in docs/misra/deviations.rst. I would leave Rule 17.1 without
> > a note.
> 
> I can see your point, and I don't have a good suggestion on possible text.
> Still I wouldn't feel well ack-ing this in its present shape.

What about:

 - It is understood that in some limited circumstances  is
   appropriate to use, such as the implementation of printk. Those
   cases will be dealt with using deviations as usual, see
   docs/misra/deviations.rst and
   docs/misra/documenting-violations.rst.



> >>> @@ -478,12 +490,24 @@ maintainers if you want to suggest a change.
> >>> have an explicit return statement with an expression
> >>>   -
> >>>  
> >>> +   * - `Rule 17.5 
> >>> `_
> >>> + - Advisory
> >>> + - The function argument corresponding to a parameter declared to
> >>> +   have an array type shall have an appropriate number of elements
> >>> + -
> >>> +
> >>> * - `Rule 17.6 
> >>> `_
> >>>   - Mandatory
> >>>   - The declaration of an array parameter shall not contain the
> >>> static keyword between the [ ]
> >>>   -
> >>>  
> >>> +   * - `Rule 17.7 
> >>> `_
> >>> + - Required
> >>> + - The value returned by a function having non-void return type
> >>> +   shall be used
> >>> + -
> >>
> >> Same question here.
> > 
> > Here I was also thinking it might be good to add a comment. Maybe we could
> > add:
> > 
> >  - Please beware that this rule has many violations in the Xen
> >codebase today, and its adoption is aspirational. However, when
> >submitting new patches please try to decrease the number of
> >violations when possible.
> 
> Yea, I think this would be good to add.

I sent out a patch with this addition, and removing Rule 17.1 as that
one is a bit more complicated.

Re: [PATCH] docs/misra/rules.rst: add more rules

[Jan Beulich]

On 07.12.2023 03:42, Stefano Stabellini wrote:
> On Wed, 6 Dec 2023, Jan Beulich wrote:
>> On 06.12.2023 04:02, Stefano Stabellini wrote:
>>> --- a/docs/misra/rules.rst
>>> +++ b/docs/misra/rules.rst
>>> @@ -462,11 +462,23 @@ maintainers if you want to suggest a change.
>>>  
>>> while(0) and while(1) and alike are allowed.
>>>  
>>> +   * - `Rule 16.3 
>>> `_
>>> + - Required
>>> + - An unconditional break statement shall terminate every
>>> +   switch-clause
>>> + - In addition to break, also other flow control statements such as
>>> +   continue, return, goto are allowed.
>>> +
>>> * - `Rule 16.7 
>>> `_
>>>   - Required
>>>   - A switch-expression shall not have essentially Boolean type
>>>   -
>>>  
>>> +   * - `Rule 17.1 
>>> `_
>>> + - Required
>>> + - The features of  shall not be used
>>> + -
>>
>> Did we really accept this without any constraint (warranting mentioning
>> here)?
> 
> We agreed that in certain situations stdarg.h is OK to use and in those
> cases we would add a deviation. Would you like me to add something to
> that effect here? I could do that but it would sound a bit vague.  Also
> if we want to specify a project-wide deviation it would be better
> documented in docs/misra/deviations.rst. I would leave Rule 17.1 without
> a note.

I can see your point, and I don't have a good suggestion on possible text.
Still I wouldn't feel well ack-ing this in its present shape.

>>> @@ -478,12 +490,24 @@ maintainers if you want to suggest a change.
>>> have an explicit return statement with an expression
>>>   -
>>>  
>>> +   * - `Rule 17.5 
>>> `_
>>> + - Advisory
>>> + - The function argument corresponding to a parameter declared to
>>> +   have an array type shall have an appropriate number of elements
>>> + -
>>> +
>>> * - `Rule 17.6 
>>> `_
>>>   - Mandatory
>>>   - The declaration of an array parameter shall not contain the
>>> static keyword between the [ ]
>>>   -
>>>  
>>> +   * - `Rule 17.7 
>>> `_
>>> + - Required
>>> + - The value returned by a function having non-void return type
>>> +   shall be used
>>> + -
>>
>> Same question here.
> 
> Here I was also thinking it might be good to add a comment. Maybe we could
> add:
> 
>  - Please beware that this rule has many violations in the Xen
>codebase today, and its adoption is aspirational. However, when
>submitting new patches please try to decrease the number of
>violations when possible.

Yea, I think this would be good to add.

Jan

Re: [PATCH] docs/misra/rules.rst: add more rules

[Stefano Stabellini]

On Wed, 6 Dec 2023, Jan Beulich wrote:
> On 06.12.2023 04:02, Stefano Stabellini wrote:
> > --- a/docs/misra/rules.rst
> > +++ b/docs/misra/rules.rst
> > @@ -462,11 +462,23 @@ maintainers if you want to suggest a change.
> >  
> > while(0) and while(1) and alike are allowed.
> >  
> > +   * - `Rule 16.3 
> > `_
> > + - Required
> > + - An unconditional break statement shall terminate every
> > +   switch-clause
> > + - In addition to break, also other flow control statements such as
> > +   continue, return, goto are allowed.
> > +
> > * - `Rule 16.7 
> > `_
> >   - Required
> >   - A switch-expression shall not have essentially Boolean type
> >   -
> >  
> > +   * - `Rule 17.1 
> > `_
> > + - Required
> > + - The features of  shall not be used
> > + -
> 
> Did we really accept this without any constraint (warranting mentioning
> here)?

We agreed that in certain situations stdarg.h is OK to use and in those
cases we would add a deviation. Would you like me to add something to
that effect here? I could do that but it would sound a bit vague.  Also
if we want to specify a project-wide deviation it would be better
documented in docs/misra/deviations.rst. I would leave Rule 17.1 without
a note.


> > @@ -478,12 +490,24 @@ maintainers if you want to suggest a change.
> > have an explicit return statement with an expression
> >   -
> >  
> > +   * - `Rule 17.5 
> > `_
> > + - Advisory
> > + - The function argument corresponding to a parameter declared to
> > +   have an array type shall have an appropriate number of elements
> > + -
> > +
> > * - `Rule 17.6 
> > `_
> >   - Mandatory
> >   - The declaration of an array parameter shall not contain the
> > static keyword between the [ ]
> >   -
> >  
> > +   * - `Rule 17.7 
> > `_
> > + - Required
> > + - The value returned by a function having non-void return type
> > +   shall be used
> > + -
> 
> Same question here.

Here I was also thinking it might be good to add a comment. Maybe we could
add:

 - Please beware that this rule has many violations in the Xen
   codebase today, and its adoption is aspirational. However, when
   submitting new patches please try to decrease the number of
   violations when possible.

I would also mention a GCC warning to use for this but I couldn't find
the right one. It looks like all the -Wunused warnings do something
different.


> If the other additions were separate, I probably would have ack-ed them
> right away.

If we can't find the right wording to use quickly I can separate them
out

Re: [PATCH] docs/misra/rules.rst: add more rules

[Jan Beulich]

On 06.12.2023 04:02, Stefano Stabellini wrote:
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -462,11 +462,23 @@ maintainers if you want to suggest a change.
>  
> while(0) and while(1) and alike are allowed.
>  
> +   * - `Rule 16.3 
> `_
> + - Required
> + - An unconditional break statement shall terminate every
> +   switch-clause
> + - In addition to break, also other flow control statements such as
> +   continue, return, goto are allowed.
> +
> * - `Rule 16.7 
> `_
>   - Required
>   - A switch-expression shall not have essentially Boolean type
>   -
>  
> +   * - `Rule 17.1 
> `_
> + - Required
> + - The features of  shall not be used
> + -

Did we really accept this without any constraint (warranting mentioning
here)?

> @@ -478,12 +490,24 @@ maintainers if you want to suggest a change.
> have an explicit return statement with an expression
>   -
>  
> +   * - `Rule 17.5 
> `_
> + - Advisory
> + - The function argument corresponding to a parameter declared to
> +   have an array type shall have an appropriate number of elements
> + -
> +
> * - `Rule 17.6 
> `_
>   - Mandatory
>   - The declaration of an array parameter shall not contain the
> static keyword between the [ ]
>   -
>  
> +   * - `Rule 17.7 
> `_
> + - Required
> + - The value returned by a function having non-void return type
> +   shall be used
> + -

Same question here.

If the other additions were separate, I probably would have ack-ed them
right away.

Jan

[PATCH] docs/misra/rules.rst: add more rules

[Stefano Stabellini]

Add the rules accepted in the last three MISRA C working group meetings.

Signed-off-by: Stefano Stabellini 

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 75921b9a34..ab89116a43 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -462,11 +462,23 @@ maintainers if you want to suggest a change.
 
while(0) and while(1) and alike are allowed.
 
+   * - `Rule 16.3 
`_
+ - Required
+ - An unconditional break statement shall terminate every
+   switch-clause
+ - In addition to break, also other flow control statements such as
+   continue, return, goto are allowed.
+
* - `Rule 16.7 
`_
  - Required
  - A switch-expression shall not have essentially Boolean type
  -
 
+   * - `Rule 17.1 
`_
+ - Required
+ - The features of  shall not be used
+ -
+
* - `Rule 17.3 
`_
  - Mandatory
  - A function shall not be declared implicitly
@@ -478,12 +490,24 @@ maintainers if you want to suggest a change.
have an explicit return statement with an expression
  -
 
+   * - `Rule 17.5 
`_
+ - Advisory
+ - The function argument corresponding to a parameter declared to
+   have an array type shall have an appropriate number of elements
+ -
+
* - `Rule 17.6 
`_
  - Mandatory
  - The declaration of an array parameter shall not contain the
static keyword between the [ ]
  -
 
+   * - `Rule 17.7 
`_
+ - Required
+ - The value returned by a function having non-void return type
+   shall be used
+ -
+
* - `Rule 18.3 
`_
  - Required
  - The relational operators > >= < and <= shall not be applied to objects 
of pointer type except where they point into the same object
@@ -498,6 +522,11 @@ maintainers if you want to suggest a change.
instances where Eclair is unable to verify that the code is valid
in regard to Rule 19.1. Caution reports are not violations.
 
+   * - `Rule 20.4 
`_
+ - Required
+ - A macro shall not be defined with the same name as a keyword
+ -
+
* - `Rule 20.7 
`_
  - Required
  - Expressions resulting from the expansion of macro parameters
@@ -506,6 +535,13 @@ maintainers if you want to suggest a change.
as function arguments, as macro arguments, array indices, lhs in
assignments
 
+   * - `Rule 20.9 
`_
+ - Required
+ - All identifiers used in the controlling expression of #if or
+   #elif preprocessing directives shall be #define'd before
+   evaluation
+ -
+
* - `Rule 20.13 
`_
  - Required
  - A line whose first token is # shall be a valid preprocessing