[PATCH v2] docs/misra: add exceptions to rules

Stefano Stabellini posted 1 patch 8 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230822013014.2523202-1-sstabellini@kernel.org
docs/misra/rules.rst | 36 +++++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)
[PATCH v2] docs/misra: add exceptions to rules
Posted by Stefano Stabellini 8 months, 1 week ago
From: Stefano Stabellini <stefano.stabellini@amd.com>

During the discussions that led to the acceptable of the Rules, we
decided on a few exceptions that were not properly recorded in
rules.rst. Other times, the exceptions were decided later when it came
to enabling a rule in ECLAIR.

Either way, update rules.rst with appropriate notes.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
v2:
- remove autogenerated from D4.10
- remove R2.1
- remove R5.6
- remove R7.1
- reword R8.3
---
 docs/misra/rules.rst | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 8f0e4d3f25..62bd4620fd 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -59,7 +59,8 @@ maintainers if you want to suggest a change.
      - Required
      - Precautions shall be taken in order to prevent the contents of a
        header file being included more than once
-     -
+     - Files that are intended to be included more than once do not need to
+       conform to the directive
 
    * - `Dir 4.11 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_11.c>`_
      - Required
@@ -117,7 +131,7 @@ maintainers if you want to suggest a change.
      - Required
      - The character sequences /* and // shall not be used within a
        comment
-     -
+     - Comments containing hyperlinks inside C-style block comments are safe
 
    * - `Rule 3.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_03_02.c>`_
      - Required
@@ -239,13 +256,16 @@ maintainers if you want to suggest a change.
      - Required
      - All declarations of an object or function shall use the same
        names and type qualifiers
-     -
+     - The type ret_t maybe be deliberately used and defined as int or
+       long depending on the type of guest to service
 
    * - `Rule 8.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_04.c>`_
      - Required
      - A compatible declaration shall be visible when an object or
        function with external linkage is defined
-     -
+     - Allowed exceptions: asm-offsets.c (definitions for asm modules
+       not called from C code), gcov_base.c (definitions only used in
+       non-release builds)
 
    * - `Rule 8.5 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_05_2.c>`_
      - Required
@@ -369,7 +389,9 @@ maintainers if you want to suggest a change.
      - Required
      - Expressions resulting from the expansion of macro parameters
        shall be enclosed in parentheses
-     -
+     - Extra parentheses are not required when macro parameters are used
+       as function arguments, as macro arguments, array indices, lhs in
+       assignments
 
    * - `Rule 20.13 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_20_13.c>`_
      - Required
-- 
2.25.1
Re: [PATCH v2] docs/misra: add exceptions to rules
Posted by Julien Grall 8 months, 1 week ago
Hi Stefano,

On 22/08/2023 02:30, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> During the discussions that led to the acceptable of the Rules, we
> decided on a few exceptions that were not properly recorded in
> rules.rst. Other times, the exceptions were decided later when it came
> to enabling a rule in ECLAIR.
> 
> Either way, update rules.rst with appropriate notes.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
> v2:
> - remove autogenerated from D4.10
> - remove R2.1
> - remove R5.6
> - remove R7.1
> - reword R8.3
> ---
>   docs/misra/rules.rst | 36 +++++++++++++++++++++++++++++-------
>   1 file changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> index 8f0e4d3f25..62bd4620fd 100644
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -59,7 +59,8 @@ maintainers if you want to suggest a change.
>        - Required
>        - Precautions shall be taken in order to prevent the contents of a
>          header file being included more than once
> -     -

It is not clear to me why this line is removed. Was it added by mistake 
in a previous commit?

> +     - Files that are intended to be included more than once do not need to
> +       conform to the directive
>   
>      * - `Dir 4.11 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_11.c>`_
>        - Required
> @@ -117,7 +131,7 @@ maintainers if you want to suggest a change.
>        - Required
>        - The character sequences /* and // shall not be used within a
>          comment
> -     -
> +     - Comments containing hyperlinks inside C-style block comments are safe
>   
>      * - `Rule 3.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_03_02.c>`_
>        - Required
> @@ -239,13 +256,16 @@ maintainers if you want to suggest a change.
>        - Required
>        - All declarations of an object or function shall use the same
>          names and type qualifiers
> -     -
> +     - The type ret_t maybe be deliberately used and defined as int or
> +       long depending on the type of guest to service
>   
>      * - `Rule 8.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_04.c>`_
>        - Required
>        - A compatible declaration shall be visible when an object or
>          function with external linkage is defined
> -     -
> +     - Allowed exceptions: asm-offsets.c (definitions for asm modules
> +       not called from C code), gcov_base.c (definitions only used in
> +       non-release builds)
>   
>      * - `Rule 8.5 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_05_2.c>`_
>        - Required
> @@ -369,7 +389,9 @@ maintainers if you want to suggest a change.
>        - Required
>        - Expressions resulting from the expansion of macro parameters
>          shall be enclosed in parentheses
> -     -
> +     - Extra parentheses are not required when macro parameters are used
> +       as function arguments, as macro arguments, array indices, lhs in
> +       assignments
>   
>      * - `Rule 20.13 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_20_13.c>`_
>        - Required

-- 
Julien Grall
Re: [PATCH v2] docs/misra: add exceptions to rules
Posted by Jan Beulich 8 months, 1 week ago
On 22.08.2023 09:56, Julien Grall wrote:
> On 22/08/2023 02:30, Stefano Stabellini wrote:
>> --- a/docs/misra/rules.rst
>> +++ b/docs/misra/rules.rst
>> @@ -59,7 +59,8 @@ maintainers if you want to suggest a change.
>>        - Required
>>        - Precautions shall be taken in order to prevent the contents of a
>>          header file being included more than once
>> -     -
> 
> It is not clear to me why this line is removed. Was it added by mistake 
> in a previous commit?

The patch is replacing an empty comment ...

>> +     - Files that are intended to be included more than once do not need to
>> +       conform to the directive

... with a non-empty one.

Jan
Re: [PATCH v2] docs/misra: add exceptions to rules
Posted by Julien Grall 8 months, 1 week ago
Hi Jan,

On 22/08/2023 11:12, Jan Beulich wrote:
> On 22.08.2023 09:56, Julien Grall wrote:
>> On 22/08/2023 02:30, Stefano Stabellini wrote:
>>> --- a/docs/misra/rules.rst
>>> +++ b/docs/misra/rules.rst
>>> @@ -59,7 +59,8 @@ maintainers if you want to suggest a change.
>>>         - Required
>>>         - Precautions shall be taken in order to prevent the contents of a
>>>           header file being included more than once
>>> -     -
>>
>> It is not clear to me why this line is removed. Was it added by mistake
>> in a previous commit?
> 
> The patch is replacing an empty comment ...
> 
>>> +     - Files that are intended to be included more than once do not need to
>>> +       conform to the directive
> 
> ... with a non-empty one.

I understand that... My question is more related to why we originally 
added one? If this was done on purpose, then why are we removing it?

Cheers,

-- 
Julien Grall
Re: [PATCH v2] docs/misra: add exceptions to rules
Posted by Jan Beulich 8 months, 1 week ago
On 22.08.2023 12:33, Julien Grall wrote:
> Hi Jan,
> 
> On 22/08/2023 11:12, Jan Beulich wrote:
>> On 22.08.2023 09:56, Julien Grall wrote:
>>> On 22/08/2023 02:30, Stefano Stabellini wrote:
>>>> --- a/docs/misra/rules.rst
>>>> +++ b/docs/misra/rules.rst
>>>> @@ -59,7 +59,8 @@ maintainers if you want to suggest a change.
>>>>         - Required
>>>>         - Precautions shall be taken in order to prevent the contents of a
>>>>           header file being included more than once
>>>> -     -
>>>
>>> It is not clear to me why this line is removed. Was it added by mistake
>>> in a previous commit?
>>
>> The patch is replacing an empty comment ...
>>
>>>> +     - Files that are intended to be included more than once do not need to
>>>> +       conform to the directive
>>
>> ... with a non-empty one.
> 
> I understand that... My question is more related to why we originally 
> added one? If this was done on purpose, then why are we removing it?

I think the goal is for the file to be easily machine readable, and hence
the same number of sub-items want to appear for every main item.

Jan
Re: [PATCH v2] docs/misra: add exceptions to rules
Posted by Stefano Stabellini 8 months, 1 week ago
On Tue, 22 Aug 2023, Jan Beulich wrote:
> On 22.08.2023 12:33, Julien Grall wrote:
> > Hi Jan,
> > 
> > On 22/08/2023 11:12, Jan Beulich wrote:
> >> On 22.08.2023 09:56, Julien Grall wrote:
> >>> On 22/08/2023 02:30, Stefano Stabellini wrote:
> >>>> --- a/docs/misra/rules.rst
> >>>> +++ b/docs/misra/rules.rst
> >>>> @@ -59,7 +59,8 @@ maintainers if you want to suggest a change.
> >>>>         - Required
> >>>>         - Precautions shall be taken in order to prevent the contents of a
> >>>>           header file being included more than once
> >>>> -     -
> >>>
> >>> It is not clear to me why this line is removed. Was it added by mistake
> >>> in a previous commit?
> >>
> >> The patch is replacing an empty comment ...
> >>
> >>>> +     - Files that are intended to be included more than once do not need to
> >>>> +       conform to the directive
> >>
> >> ... with a non-empty one.
> > 
> > I understand that... My question is more related to why we originally 
> > added one? If this was done on purpose, then why are we removing it?
> 
> I think the goal is for the file to be easily machine readable, and hence
> the same number of sub-items want to appear for every main item.

Yes, it is to respect the RST table format so that gets rendered as a
table in HTML/PDF/etc.
Re: [PATCH v2] docs/misra: add exceptions to rules
Posted by Jan Beulich 8 months, 1 week ago
On 22.08.2023 03:30, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> During the discussions that led to the acceptable of the Rules, we

Nit: acceptance

> decided on a few exceptions that were not properly recorded in
> rules.rst. Other times, the exceptions were decided later when it came
> to enabling a rule in ECLAIR.
> 
> Either way, update rules.rst with appropriate notes.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>[...]
> @@ -239,13 +256,16 @@ maintainers if you want to suggest a change.
>       - Required
>       - All declarations of an object or function shall use the same
>         names and type qualifiers
> -     -
> +     - The type ret_t maybe be deliberately used and defined as int or
> +       long depending on the type of guest to service
>  
>     * - `Rule 8.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_04.c>`_
>       - Required
>       - A compatible declaration shall be visible when an object or
>         function with external linkage is defined
> -     -
> +     - Allowed exceptions: asm-offsets.c (definitions for asm modules
> +       not called from C code), gcov_base.c (definitions only used in
> +       non-release builds)

Doesn't this want to be

     - Allowed exceptions: asm-offsets.c, definitions for asm modules
       not called from C code, and gcov_base.c (definitions only used in
       non-release builds)

? As to coverage: Why "only used in non-release builds"? COVERAGE doesn't
depend on DEBUG, and people may actually want to enable it for release
builds. Just likely not for production ones.

If you agree with and make respective adjustments:
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan
Re: [PATCH v2] docs/misra: add exceptions to rules
Posted by Stefano Stabellini 8 months, 1 week ago
On Tue, 22 Aug 2023, Jan Beulich wrote:
> On 22.08.2023 03:30, Stefano Stabellini wrote:
> > From: Stefano Stabellini <stefano.stabellini@amd.com>
> > 
> > During the discussions that led to the acceptable of the Rules, we
> 
> Nit: acceptance
> 
> > decided on a few exceptions that were not properly recorded in
> > rules.rst. Other times, the exceptions were decided later when it came
> > to enabling a rule in ECLAIR.
> > 
> > Either way, update rules.rst with appropriate notes.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> >[...]
> > @@ -239,13 +256,16 @@ maintainers if you want to suggest a change.
> >       - Required
> >       - All declarations of an object or function shall use the same
> >         names and type qualifiers
> > -     -
> > +     - The type ret_t maybe be deliberately used and defined as int or
> > +       long depending on the type of guest to service
> >  
> >     * - `Rule 8.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_04.c>`_
> >       - Required
> >       - A compatible declaration shall be visible when an object or
> >         function with external linkage is defined
> > -     -
> > +     - Allowed exceptions: asm-offsets.c (definitions for asm modules
> > +       not called from C code), gcov_base.c (definitions only used in
> > +       non-release builds)
> 
> Doesn't this want to be
> 
>      - Allowed exceptions: asm-offsets.c, definitions for asm modules
>        not called from C code, and gcov_base.c (definitions only used in
>        non-release builds)
> 
> ? As to coverage: Why "only used in non-release builds"? COVERAGE doesn't
> depend on DEBUG, and people may actually want to enable it for release
> builds. Just likely not for production ones.
> 
> If you agree with and make respective adjustments:
> Acked-by: Jan Beulich <jbeulich@suse.com>

Yes I agree to the changes and I made them on commit