| Message ID | 20230823002458.2738365-1-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | docs/misra: add rule 2.1 exceptions | expand |
On 23.08.2023 02:24, Stefano Stabellini wrote: > From: Stefano Stabellini <stefano.stabellini@amd.com> > > During the discussions that led to the acceptable of Rule 2.1, we Nit (as before): "acceptance"? > 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> > --- > 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 b6d87e076b..8f38227994 100644 > --- a/docs/misra/rules.rst > +++ b/docs/misra/rules.rst > @@ -107,7 +107,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 never referenced from C > + code (e.g. 'do_trap_fiq') Maybe better put it the other way around, "only referenced from assembly code"? This is because "never referenced from C" also includes truly unreferenced functions/data. > + - Unreachability caused by the certain macros/functions is > + deliberate, e.g. BUG, assert_failed, panic, etc. I think the "the" here wants dropping, and even then the result doesn't read very well imo. How about "Deliberate unreachability caused by certain macros/functions, e.g. BUG, assert_failed, panic, etc"? > + - 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 Provided additionally this sub-sub-bullet-list then also translates correctly to the various derived formats, then: Acked-by: Jan Beulich <jbeulich@suse.com> Jan
Hi Stefano, On 23/08/2023 01:24, Stefano Stabellini wrote: > From: Stefano Stabellini <stefano.stabellini@amd.com> > > During the discussions that led to the acceptable 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> > --- > 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 b6d87e076b..8f38227994 100644 > --- a/docs/misra/rules.rst > +++ b/docs/misra/rules.rst > @@ -107,7 +107,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 never referenced from C > + code (e.g. 'do_trap_fiq') > + - Unreachability caused by the certain macros/functions is > + deliberate, e.g. BUG, assert_failed, panic, etc. I find the wording quite ambiguous. How will an assessor be able to know this is deliberate? I think it would be better if this is based on a keyword in the code such as a function that has the attribute noreturn and/or there is an unreachable() statement. Cheers,
On Wed, 23 Aug 2023, Julien Grall wrote: > Hi Stefano, > > On 23/08/2023 01:24, Stefano Stabellini wrote: > > From: Stefano Stabellini <stefano.stabellini@amd.com> > > > > During the discussions that led to the acceptable 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> > > --- > > 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 b6d87e076b..8f38227994 100644 > > --- a/docs/misra/rules.rst > > +++ b/docs/misra/rules.rst > > @@ -107,7 +107,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 never referenced from C > > + code (e.g. 'do_trap_fiq') > > + - Unreachability caused by the certain macros/functions is > > + deliberate, e.g. BUG, assert_failed, panic, etc. > > I find the wording quite ambiguous. How will an assessor be able to know this > is deliberate? I think it would be better if this is based on a keyword in the > code such as a function that has the attribute noreturn and/or there is an > unreachable() statement. You are right that it is not precise enough. I am thinking of changing this to (also following Jan's suggestion): Deliberate unreachability caused by certain macros/functions, e.g. BUG, assert_failed, panic, etc. See safe.json. In particular the important part is "See safe.json". Right now there is nothing in safe.json about it, but it is should be the right place to maintain the list of deviations and tags. Then we can add the tag at the declaration site of BUG, panic, etc. Nicola is also checking if this approach works with ECLAIR.
On Wed, 23 Aug 2023, Jan Beulich wrote: > On 23.08.2023 02:24, Stefano Stabellini wrote: > > From: Stefano Stabellini <stefano.stabellini@amd.com> > > > > During the discussions that led to the acceptable of Rule 2.1, we > > Nit (as before): "acceptance"? > > > 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> > > --- > > 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 b6d87e076b..8f38227994 100644 > > --- a/docs/misra/rules.rst > > +++ b/docs/misra/rules.rst > > @@ -107,7 +107,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 never referenced from C > > + code (e.g. 'do_trap_fiq') > > Maybe better put it the other way around, "only referenced from assembly > code"? This is because "never referenced from C" also includes truly > unreferenced functions/data. > > > + - Unreachability caused by the certain macros/functions is > > + deliberate, e.g. BUG, assert_failed, panic, etc. > > I think the "the" here wants dropping, and even then the result doesn't > read very well imo. How about "Deliberate unreachability caused by > certain macros/functions, e.g. BUG, assert_failed, panic, etc"? > > > + - 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 > > Provided additionally this sub-sub-bullet-list then also translates > correctly to the various derived formats, then: > Acked-by: Jan Beulich <jbeulich@suse.com> Yes I checked. Also thanks for the good suggestions, I'll make the changes and resend.
diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst index b6d87e076b..8f38227994 100644 --- a/docs/misra/rules.rst +++ b/docs/misra/rules.rst @@ -107,7 +107,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 never referenced from C + code (e.g. 'do_trap_fiq') + - Unreachability caused by the certain macros/functions is + deliberate, e.g. BUG, assert_failed, panic, etc. + - 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