| Message ID | 20230822013014.2523202-1-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] docs/misra: add exceptions to rules | expand |
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
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
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
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,
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
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.
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
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