[PATCH v2] docs/misra: add rule 2.1 exceptions

Stefano Stabellini posted 1 patch 8 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230823223942.2981782-1-sstabellini@kernel.org
There is a newer version of this series
docs/misra/rules.rst | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
[PATCH v2] docs/misra: add rule 2.1 exceptions
Posted by Stefano Stabellini 8 months, 2 weeks ago
From: Stefano Stabellini <stefano.stabellini@amd.com>

During the discussions that led to the acceptance of Rule 2.1, we
decided on a few exceptions that were not properly recorded in
rules.rst. Add them now.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Note that safe.json and the codebase are not yet updated with an
appropriate tag for BUG, panic and friends.

v2:
- fix typo in commit message
- use "only referenced from assembly"
- use "Deliberate unreachability caused by"
- add "See safe.json"
- add acked-by (although I also added "See safe.json")
---
 docs/misra/rules.rst | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 8f0e4d3f25..4f33ed4ba6 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -106,7 +106,18 @@ maintainers if you want to suggest a change.
    * - `Rule 2.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_02_01_1.c>`_
      - Required
      - A project shall not contain unreachable code
-     -
+     - The following are allowed:
+         - Invariantly constant conditions, e.g. if(IS_ENABLED(CONFIG_HVM)) { S; }
+         - Switch with a controlling value statically determined not to
+           match one or more case statements
+         - Functions that are intended to be referenced only from
+           assembly code (e.g. 'do_trap_fiq')
+         - Deliberate unreachability caused by certain macros/functions,
+           e.g. BUG, assert_failed, panic, etc. See safe.json.
+         - asm-offsets.c, as they are not linked deliberately, because
+           they are used to generate definitions for asm modules
+         - Declarations without initializer are safe, as they are not
+           executed
 
    * - `Rule 2.6 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_02_06.c>`_
      - Advisory
-- 
2.25.1
Re: [PATCH v2] docs/misra: add rule 2.1 exceptions
Posted by Bertrand Marquis 7 months, 1 week ago
Hi Stefano,

> On 24 Aug 2023, at 00:39, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> From: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> During the discussions that led to the acceptance of Rule 2.1, we
> decided on a few exceptions that were not properly recorded in
> rules.rst. Add them now.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> ---
> Note that safe.json and the codebase are not yet updated with an
> appropriate tag for BUG, panic and friends.
> 
> v2:
> - fix typo in commit message
> - use "only referenced from assembly"
> - use "Deliberate unreachability caused by"
> - add "See safe.json"
> - add acked-by (although I also added "See safe.json")
> ---
> docs/misra/rules.rst | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> index 8f0e4d3f25..4f33ed4ba6 100644
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -106,7 +106,18 @@ maintainers if you want to suggest a change.
>    * - `Rule 2.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_02_01_1.c>`_
>      - Required
>      - A project shall not contain unreachable code
> -     -
> +     - The following are allowed:
> +         - Invariantly constant conditions, e.g. if(IS_ENABLED(CONFIG_HVM)) { S; }
> +         - Switch with a controlling value statically determined not to
> +           match one or more case statements
> +         - Functions that are intended to be referenced only from
> +           assembly code (e.g. 'do_trap_fiq')
> +         - Deliberate unreachability caused by certain macros/functions,
> +           e.g. BUG, assert_failed, panic, etc. See safe.json.

As Julien requested, you should remove this.

With that handled:

Acked-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> +         - asm-offsets.c, as they are not linked deliberately, because
> +           they are used to generate definitions for asm modules
> +         - Declarations without initializer are safe, as they are not
> +           executed
> 
>    * - `Rule 2.6 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_02_06.c>`_
>      - Advisory
> -- 
> 2.25.1
> 
Re: [PATCH v2] docs/misra: add rule 2.1 exceptions
Posted by Henry Wang 7 months, 1 week ago
Hi Stefano,

> On Sep 28, 2023, at 17:10, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
> 
> Hi Stefano,
> 
>> On 24 Aug 2023, at 00:39, Stefano Stabellini <sstabellini@kernel.org> wrote:
>> 
>> From: Stefano Stabellini <stefano.stabellini@amd.com>
>> 
>> During the discussions that led to the acceptance of Rule 2.1, we
>> decided on a few exceptions that were not properly recorded in
>> rules.rst. Add them now.
>> 
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Note that safe.json and the codebase are not yet updated with an
>> appropriate tag for BUG, panic and friends.
>> 
>> v2:
>> - fix typo in commit message
>> - use "only referenced from assembly"
>> - use "Deliberate unreachability caused by"
>> - add "See safe.json"
>> - add acked-by (although I also added "See safe.json")
>> ---
>> docs/misra/rules.rst | 13 ++++++++++++-
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>> 
>> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
>> index 8f0e4d3f25..4f33ed4ba6 100644
>> --- a/docs/misra/rules.rst
>> +++ b/docs/misra/rules.rst
>> @@ -106,7 +106,18 @@ maintainers if you want to suggest a change.
>>   * - `Rule 2.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_02_01_1.c>`_
>>     - Required
>>     - A project shall not contain unreachable code
>> -     -
>> +     - The following are allowed:
>> +         - Invariantly constant conditions, e.g. if(IS_ENABLED(CONFIG_HVM)) { S; }
>> +         - Switch with a controlling value statically determined not to
>> +           match one or more case statements
>> +         - Functions that are intended to be referenced only from
>> +           assembly code (e.g. 'do_trap_fiq')
>> +         - Deliberate unreachability caused by certain macros/functions,
>> +           e.g. BUG, assert_failed, panic, etc. See safe.json.
> 
> As Julien requested, you should remove this.
> 
> With that handled:
> 
> Acked-by: Bertrand Marquis <bertrand.marquis@arm.com>

With Bertrand’s comments addressed:

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry


> 
> Cheers
> Bertrand
> 
>> +         - asm-offsets.c, as they are not linked deliberately, because
>> +           they are used to generate definitions for asm modules
>> +         - Declarations without initializer are safe, as they are not
>> +           executed
>> 
>>   * - `Rule 2.6 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_02_06.c>`_
>>     - Advisory
>> -- 
>> 2.25.1
>> 
> 
> 

Re: [PATCH v2] docs/misra: add rule 2.1 exceptions
Posted by Julien Grall 8 months, 2 weeks ago

On 23/08/2023 23:39, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> During the discussions that led to the acceptance of Rule 2.1, we
> decided on a few exceptions that were not properly recorded in
> rules.rst. Add them now.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> ---
> Note that safe.json and the codebase are not yet updated with an
> appropriate tag for BUG, panic and friends.

I think it should be updated with at least one of them. Otherwise...

> 
> v2:
> - fix typo in commit message
> - use "only referenced from assembly"
> - use "Deliberate unreachability caused by"
> - add "See safe.json"
> - add acked-by (although I also added "See safe.json")
> ---
>   docs/misra/rules.rst | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> index 8f0e4d3f25..4f33ed4ba6 100644
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -106,7 +106,18 @@ maintainers if you want to suggest a change.
>      * - `Rule 2.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_02_01_1.c>`_
>        - Required
>        - A project shall not contain unreachable code
> -     -
> +     - The following are allowed:
> +         - Invariantly constant conditions, e.g. if(IS_ENABLED(CONFIG_HVM)) { S; }
> +         - Switch with a controlling value statically determined not to
> +           match one or more case statements
> +         - Functions that are intended to be referenced only from
> +           assembly code (e.g. 'do_trap_fiq')
> +         - Deliberate unreachability caused by certain macros/functions,
> +           e.g. BUG, assert_failed, panic, etc. See safe.json.

... someone reading this and then reading safe.json will wonder why none 
are present.

The list would then only contain the one(s) currently added in 
safe.json. But there should be no expectation that the examples will 
grow everytime one is added in safe.json.

> +         - asm-offsets.c, as they are not linked deliberately, because
> +           they are used to generate definitions for asm modules
> +         - Declarations without initializer are safe, as they are not
> +           executed
>   
>      * - `Rule 2.6 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_02_06.c>`_
>        - Advisory

Cheers,

-- 
Julien Grall
Re: [PATCH v2] docs/misra: add rule 2.1 exceptions
Posted by Julien Grall 8 months, 2 weeks ago
On 24/08/2023 12:46, Julien Grall wrote:
> On 23/08/2023 23:39, Stefano Stabellini wrote:
>> ---
>>   docs/misra/rules.rst | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
>> index 8f0e4d3f25..4f33ed4ba6 100644
>> --- a/docs/misra/rules.rst
>> +++ b/docs/misra/rules.rst
>> @@ -106,7 +106,18 @@ maintainers if you want to suggest a change.
>>      * - `Rule 2.1 
>> <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_02_01_1.c>`_
>>        - Required
>>        - A project shall not contain unreachable code
>> -     -
>> +     - The following are allowed:
>> +         - Invariantly constant conditions, e.g. 
>> if(IS_ENABLED(CONFIG_HVM)) { S; }
>> +         - Switch with a controlling value statically determined not to
>> +           match one or more case statements
>> +         - Functions that are intended to be referenced only from
>> +           assembly code (e.g. 'do_trap_fiq')
>> +         - Deliberate unreachability caused by certain macros/functions,
>> +           e.g. BUG, assert_failed, panic, etc. See safe.json.
> 
> ... someone reading this and then reading safe.json will wonder why none 
> are present.
> 
> The list would then only contain the one(s) currently added in 
> safe.json. But there should be no expectation that the examples will 
> grow everytime one is added in safe.json.

I forgot to mention that the rest of the exceptions look good to me. So 
if you prefer, if you could remove this exception and commit the rest.

Cheers,

-- 
Julien Grall

Re: [PATCH v2] docs/misra: add rule 2.1 exceptions
Posted by Stefano Stabellini 8 months, 2 weeks ago
On Thu, 24 Aug 2023, Julien Grall wrote:
> On 24/08/2023 12:46, Julien Grall wrote:
> > On 23/08/2023 23:39, Stefano Stabellini wrote:
> > > ---
> > >   docs/misra/rules.rst | 13 ++++++++++++-
> > >   1 file changed, 12 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> > > index 8f0e4d3f25..4f33ed4ba6 100644
> > > --- a/docs/misra/rules.rst
> > > +++ b/docs/misra/rules.rst
> > > @@ -106,7 +106,18 @@ maintainers if you want to suggest a change.
> > >      * - `Rule 2.1
> > > <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_02_01_1.c>`_
> > >        - Required
> > >        - A project shall not contain unreachable code
> > > -     -
> > > +     - The following are allowed:
> > > +         - Invariantly constant conditions, e.g.
> > > if(IS_ENABLED(CONFIG_HVM)) { S; }
> > > +         - Switch with a controlling value statically determined not to
> > > +           match one or more case statements
> > > +         - Functions that are intended to be referenced only from
> > > +           assembly code (e.g. 'do_trap_fiq')
> > > +         - Deliberate unreachability caused by certain macros/functions,
> > > +           e.g. BUG, assert_failed, panic, etc. See safe.json.
> > 
> > ... someone reading this and then reading safe.json will wonder why none are
> > present.
> > 
> > The list would then only contain the one(s) currently added in safe.json.
> > But there should be no expectation that the examples will grow everytime one
> > is added in safe.json.
> 
> I forgot to mention that the rest of the exceptions look good to me. So if you
> prefer, if you could remove this exception and commit the rest.

Let me see what I can do