docs/misra/rules.rst | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-)
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>
---
Note that there might be more to add, but the below look correct to me
---
docs/misra/rules.rst | 41 +++++++++++++++++++++++++++++++++--------
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 8f0e4d3f25..ecbb04da96 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 (e.g. autogenerated or empty header files)
* - `Dir 4.11 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_11.c>`_
- Required
@@ -106,7 +107,23 @@ 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. while(0) { S; })
+ - Switch with a controlling value incompatible with labeled
+ statements
+ - Functions that are intended to be never referenced from C
+ code, or are referenced in builds not under analysis (e.g.
+ 'do_trap_fiq' for the former and 'check_for_unexpected_msi'
+ for the latter)
+ - Unreachability caused by the following macros/functions is
+ deliberate: BUG, assert_failed, ERROR_EXIT, ERROR_EXIT_DOM,
+ PIN_FAIL, __builtin_unreachable, panic, do_unexpected_trap,
+ machine_halt, machine_restart, machine_reboot,
+ ASSERT_UNREACHABLE
+ - asm-offsets.c, as they are not linked deliberately, because
+ they are used to generate definitions for asm modules
+ - pure declarations (i.e. declarations without
+ initialization) 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
@@ -117,7 +134,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
@@ -167,7 +184,7 @@ maintainers if you want to suggest a change.
* - `Rule 5.6 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_05_06.c>`_
- Required
- A typedef name shall be a unique identifier
- -
+ - BOOLEAN, UINT{8,32,64} and INT{8,32,64} are allowed
* - `Rule 6.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_01.c>`_
- Required
@@ -183,7 +200,10 @@ maintainers if you want to suggest a change.
* - `Rule 7.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_01.c>`_
- Required
- Octal constants shall not be used
- -
+ - Usage of the following constants is safe, since they are given
+ as-is in the inflate algorithm specification and there is
+ therefore no risk of them being interpreted as decimal constants:
+ ^0(007|37|070|213|236|300|321|330|331|332|333|334|335|337|371)$
* - `Rule 7.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_
- Required
@@ -239,13 +259,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 is deliberately used and defined as int or long
+ depending on the architecture
* - `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 +392,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.1On 19.08.2023 03:24, 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.
In a number of cases I'm unaware of such decisions. May be worth splitting
the patch into a controversial and an uncontroversial part, such that the
latter can go in while we discuss the former.
> --- 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 (e.g. autogenerated or empty header files)
Auto-generated isn't a reason for an exception here. The logic generating
the header can very well be adjusted. Same for empty headers - there's no
reason they couldn't gain guards. An exception is needed for headers which
we deliberately include more than once, in order to have a single central
place for attributes, enumerations, and alike.
> @@ -106,7 +107,23 @@ 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. while(0) { S; })
When (and why) was this decided? The example given looks exactly like what
Misra wants us to not have.
> + - Switch with a controlling value incompatible with labeled
> + statements
What does this mean?
> + - Functions that are intended to be never referenced from C
> + code, or are referenced in builds not under analysis (e.g.
> + 'do_trap_fiq' for the former and 'check_for_unexpected_msi'
> + for the latter)
I agree with the "not referenced from C", but I don't see why the other
kind couldn't be properly addressed.
> + - Unreachability caused by the following macros/functions is
> + deliberate: BUG, assert_failed, ERROR_EXIT, ERROR_EXIT_DOM,
> + PIN_FAIL, __builtin_unreachable, panic, do_unexpected_trap,
> + machine_halt, machine_restart, machine_reboot,
> + ASSERT_UNREACHABLE
Base infrastructure items like BUG() are imo fine to mention here. But
specific items shouldn't be; the more we mention here, the more we invite
the list to be grown. Note also how you mention items which no longer
exist (ERROR_EXIT{,_DOM}, PIN_FAIL).
> + - asm-offsets.c, as they are not linked deliberately, because
> + they are used to generate definitions for asm modules
> + - pure declarations (i.e. declarations without
> + initialization) are safe, as they are not executed
I don't think "pure declarations" is a term used in the spec. Let's better
call it the way it is called elsewhere - declarations without initializer
(where, as mentioned before, the term "unreachable code" is questionable
anyway).
> @@ -167,7 +184,7 @@ maintainers if you want to suggest a change.
> * - `Rule 5.6 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_05_06.c>`_
> - Required
> - A typedef name shall be a unique identifier
> - -
> + - BOOLEAN, UINT{8,32,64} and INT{8,32,64} are allowed
I think this permission needs to be limited as much as possible.
> @@ -183,7 +200,10 @@ maintainers if you want to suggest a change.
> * - `Rule 7.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_01.c>`_
> - Required
> - Octal constants shall not be used
> - -
> + - Usage of the following constants is safe, since they are given
> + as-is in the inflate algorithm specification and there is
> + therefore no risk of them being interpreted as decimal constants:
> + ^0(007|37|070|213|236|300|321|330|331|332|333|334|335|337|371)$
This is a very odd set of exceptions, which by stating them here you then
grant to be exercised everywhere. Once again I don't think special cases
dealing with a single source or sub-component should be globally named.
> @@ -239,13 +259,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 is deliberately used and defined as int or long
> + depending on the architecture
That's not depending on the architecture, but depending on the type of
guest to service. I'd also suggest "may", not "is".
Jan
On Mon, 21 Aug 2023, Jan Beulich wrote:
> On 19.08.2023 03:24, 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.
>
> In a number of cases I'm unaware of such decisions. May be worth splitting
> the patch into a controversial and an uncontroversial part, such that the
> latter can go in while we discuss the former.
yes, I'll extract a couple of non-controversial changes and send them
out
> > --- 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 (e.g. autogenerated or empty header files)
>
> Auto-generated isn't a reason for an exception here. The logic generating
> the header can very well be adjusted. Same for empty headers - there's no
> reason they couldn't gain guards. An exception is needed for headers which
> we deliberately include more than once, in order to have a single central
> place for attributes, enumerations, and alike.
OK
> > @@ -106,7 +107,23 @@ 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. while(0) { S; })
>
> When (and why) was this decided? The example given looks exactly like what
> Misra wants us to not have.
This covers things like:
if (IS_ENABLED(CONFIG_HVM))
which could resolve in if (0) in certain configurations. I think we gave
feedback to Roberto that we wanted to keep this type of things as is.
> > + - Switch with a controlling value incompatible with labeled
> > + statements
>
> What does this mean?
I am not certain about this one actually. It could be when we have:
switch (var) {
case 1:
something();
break;
case 2:
something();
break;
}
and var could be 3 in theory?
Nicola, please confirm.
> > + - Functions that are intended to be never referenced from C
> > + code, or are referenced in builds not under analysis (e.g.
> > + 'do_trap_fiq' for the former and 'check_for_unexpected_msi'
> > + for the latter)
>
> I agree with the "not referenced from C", but I don't see why the other
> kind couldn't be properly addressed.
I think you have a point
> > + - Unreachability caused by the following macros/functions is
> > + deliberate: BUG, assert_failed, ERROR_EXIT, ERROR_EXIT_DOM,
> > + PIN_FAIL, __builtin_unreachable, panic, do_unexpected_trap,
> > + machine_halt, machine_restart, machine_reboot,
> > + ASSERT_UNREACHABLE
>
> Base infrastructure items like BUG() are imo fine to mention here. But
> specific items shouldn't be; the more we mention here, the more we invite
> the list to be grown. Note also how you mention items which no longer
> exist (ERROR_EXIT{,_DOM}, PIN_FAIL).
The question is whether we want this list to be exhaustive (so we want
to mention everything for which we make an exception) or only an example
(in which case just BUG is fine.)
Let's say we only mention BUG. Where should we keep the exhaustive list?
Is it OK if it only lives as an ECLAIR config file under
automation/eclair_analysis? There is another very similar question
below.
BTW I think both options are OK.
If we only mention BUG, we are basically saying that as a general rule
only BUG is an exception. Then we have a longer more detailed list for
ECLAIR because in practice things are always complicated.
On the other hand if we have the full list here, then the documentation
is more precise, but it looks a bit "strange" to see such a detailed
list in this document and also we need to make sure to keep the list
up-to-date.
> > + - asm-offsets.c, as they are not linked deliberately, because
> > + they are used to generate definitions for asm modules
> > + - pure declarations (i.e. declarations without
> > + initialization) are safe, as they are not executed
>
> I don't think "pure declarations" is a term used in the spec. Let's better
> call it the way it is called elsewhere - declarations without initializer
> (where, as mentioned before, the term "unreachable code" is questionable
> anyway).
OK
> > @@ -167,7 +184,7 @@ maintainers if you want to suggest a change.
> > * - `Rule 5.6 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_05_06.c>`_
> > - Required
> > - A typedef name shall be a unique identifier
> > - -
> > + - BOOLEAN, UINT{8,32,64} and INT{8,32,64} are allowed
>
> I think this permission needs to be limited as much as possible.
Maybe we should take this out completely given that they should be
limited to efi and acpi code that is excepted anyway
> > @@ -183,7 +200,10 @@ maintainers if you want to suggest a change.
> > * - `Rule 7.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_01.c>`_
> > - Required
> > - Octal constants shall not be used
> > - -
> > + - Usage of the following constants is safe, since they are given
> > + as-is in the inflate algorithm specification and there is
> > + therefore no risk of them being interpreted as decimal constants:
> > + ^0(007|37|070|213|236|300|321|330|331|332|333|334|335|337|371)$
>
> This is a very odd set of exceptions, which by stating them here you then
> grant to be exercised everywhere. Once again I don't think special cases
> dealing with a single source or sub-component should be globally named.
Actually I agree with you there. The problem is where to put them.
Right now we have docs/misra/rules.rst with the list of accepted rules
and their special interpretations by Xen Project. We also have the
ECLAIR configuration under automation/eclair_analysis with a bunch of
ECLAIR specific config files.
Is it OK if the constants above only live under
automation/eclair_analysis and nowhere else? Or should we have another
rst document under docs/misra for special cases dealing with a single
source?
> > @@ -239,13 +259,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 is deliberately used and defined as int or long
> > + depending on the architecture
>
> That's not depending on the architecture, but depending on the type of
> guest to service. I'd also suggest "may", not "is".
OK
On 22.08.2023 03:40, Stefano Stabellini wrote:
> On Mon, 21 Aug 2023, Jan Beulich wrote:
>> On 19.08.2023 03:24, Stefano Stabellini wrote:
>>> @@ -106,7 +107,23 @@ 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. while(0) { S; })
>>
>> When (and why) was this decided? The example given looks exactly like what
>> Misra wants us to not have.
>
> This covers things like:
>
> if (IS_ENABLED(CONFIG_HVM))
>
> which could resolve in if (0) in certain configurations. I think we gave
> feedback to Roberto that we wanted to keep this type of things as is.
Ah, I see. But then perhaps mention that rather than plain 0 here? See
below as to whether a complete list of excepted constructs is wanted.
>>> + - Switch with a controlling value incompatible with labeled
>>> + statements
>>
>> What does this mean?
>
> I am not certain about this one actually. It could be when we have:
>
> switch (var) {
> case 1:
> something();
> break;
> case 2:
> something();
> break;
> }
>
> and var could be 3 in theory?
That would be a unhandled value, but no unreachable code.
>>> + - Unreachability caused by the following macros/functions is
>>> + deliberate: BUG, assert_failed, ERROR_EXIT, ERROR_EXIT_DOM,
>>> + PIN_FAIL, __builtin_unreachable, panic, do_unexpected_trap,
>>> + machine_halt, machine_restart, machine_reboot,
>>> + ASSERT_UNREACHABLE
>>
>> Base infrastructure items like BUG() are imo fine to mention here. But
>> specific items shouldn't be; the more we mention here, the more we invite
>> the list to be grown. Note also how you mention items which no longer
>> exist (ERROR_EXIT{,_DOM}, PIN_FAIL).
>
> The question is whether we want this list to be exhaustive (so we want
> to mention everything for which we make an exception) or only an example
> (in which case just BUG is fine.)
>
> Let's say we only mention BUG. Where should we keep the exhaustive list?
> Is it OK if it only lives as an ECLAIR config file under
> automation/eclair_analysis? There is another very similar question
> below.
First and foremost we need to have a single place where everything is
recorded, or where at least a pointer exists to where further details
are. As to this being the Eclair config file: Shouldn't any such
constructs rather be listed in the deviations file, such that e.g.
cppcheck can also benefit?
> BTW I think both options are OK.
>
> If we only mention BUG, we are basically saying that as a general rule
> only BUG is an exception. Then we have a longer more detailed list for
> ECLAIR because in practice things are always complicated.
>
> On the other hand if we have the full list here, then the documentation
> is more precise, but it looks a bit "strange" to see such a detailed
> list in this document and also we need to make sure to keep the list
> up-to-date.
Thing is: This list shouldn't grow very long anyway, and also better
would grow / change much over time.
>>> @@ -167,7 +184,7 @@ maintainers if you want to suggest a change.
>>> * - `Rule 5.6 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_05_06.c>`_
>>> - Required
>>> - A typedef name shall be a unique identifier
>>> - -
>>> + - BOOLEAN, UINT{8,32,64} and INT{8,32,64} are allowed
>>
>> I think this permission needs to be limited as much as possible.
>
> Maybe we should take this out completely given that they should be
> limited to efi and acpi code that is excepted anyway
I would favor that, yes.
>>> @@ -183,7 +200,10 @@ maintainers if you want to suggest a change.
>>> * - `Rule 7.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_01.c>`_
>>> - Required
>>> - Octal constants shall not be used
>>> - -
>>> + - Usage of the following constants is safe, since they are given
>>> + as-is in the inflate algorithm specification and there is
>>> + therefore no risk of them being interpreted as decimal constants:
>>> + ^0(007|37|070|213|236|300|321|330|331|332|333|334|335|337|371)$
>>
>> This is a very odd set of exceptions, which by stating them here you then
>> grant to be exercised everywhere. Once again I don't think special cases
>> dealing with a single source or sub-component should be globally named.
>
> Actually I agree with you there. The problem is where to put them.
safe.json?
> Right now we have docs/misra/rules.rst with the list of accepted rules
> and their special interpretations by Xen Project. We also have the
> ECLAIR configuration under automation/eclair_analysis with a bunch of
> ECLAIR specific config files.
>
> Is it OK if the constants above only live under
> automation/eclair_analysis and nowhere else? Or should we have another
> rst document under docs/misra for special cases dealing with a single
> source?
As per above, I think putting anything in Eclair's config file should
only ever be a last resort. Wherever possible we should try to put stuff
in files which aren't tool specific. Where necessary special tool
settings can then be (machine-)derived from there.
Jan
On Tue, 22 Aug 2023, Jan Beulich wrote:
> On 22.08.2023 03:40, Stefano Stabellini wrote:
> > On Mon, 21 Aug 2023, Jan Beulich wrote:
> >> On 19.08.2023 03:24, Stefano Stabellini wrote:
> >>> @@ -106,7 +107,23 @@ 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. while(0) { S; })
> >>
> >> When (and why) was this decided? The example given looks exactly like what
> >> Misra wants us to not have.
> >
> > This covers things like:
> >
> > if (IS_ENABLED(CONFIG_HVM))
> >
> > which could resolve in if (0) in certain configurations. I think we gave
> > feedback to Roberto that we wanted to keep this type of things as is.
>
> Ah, I see. But then perhaps mention that rather than plain 0 here?
Yes, I'll do
> See below as to whether a complete list of excepted constructs is
> wanted.
[...]
> >>> + - Unreachability caused by the following macros/functions is
> >>> + deliberate: BUG, assert_failed, ERROR_EXIT, ERROR_EXIT_DOM,
> >>> + PIN_FAIL, __builtin_unreachable, panic, do_unexpected_trap,
> >>> + machine_halt, machine_restart, machine_reboot,
> >>> + ASSERT_UNREACHABLE
> >>
> >> Base infrastructure items like BUG() are imo fine to mention here. But
> >> specific items shouldn't be; the more we mention here, the more we invite
> >> the list to be grown. Note also how you mention items which no longer
> >> exist (ERROR_EXIT{,_DOM}, PIN_FAIL).
> >
> > The question is whether we want this list to be exhaustive (so we want
> > to mention everything for which we make an exception) or only an example
> > (in which case just BUG is fine.)
> >
> > Let's say we only mention BUG. Where should we keep the exhaustive list?
> > Is it OK if it only lives as an ECLAIR config file under
> > automation/eclair_analysis? There is another very similar question
> > below.
>
> First and foremost we need to have a single place where everything is
> recorded, or where at least a pointer exists to where further details
> are. As to this being the Eclair config file: Shouldn't any such
> constructs rather be listed in the deviations file, such that e.g.
> cppcheck can also benefit?
Yes, you are right. Basically the choice is whether we want a
project-wide deviation, something like a change in how we interpret the
rules, or a regular deviation for one macro or one function.
After reading your comments, I also think that all the macros above
should be covered by safe.json and in-code comments instead.
> > BTW I think both options are OK.
> >
> > If we only mention BUG, we are basically saying that as a general rule
> > only BUG is an exception. Then we have a longer more detailed list for
> > ECLAIR because in practice things are always complicated.
> >
> > On the other hand if we have the full list here, then the documentation
> > is more precise, but it looks a bit "strange" to see such a detailed
> > list in this document and also we need to make sure to keep the list
> > up-to-date.
>
> Thing is: This list shouldn't grow very long anyway, and also better
> would grow / change much over time.
+1
> >>> @@ -167,7 +184,7 @@ maintainers if you want to suggest a change.
> >>> * - `Rule 5.6 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_05_06.c>`_
> >>> - Required
> >>> - A typedef name shall be a unique identifier
> >>> - -
> >>> + - BOOLEAN, UINT{8,32,64} and INT{8,32,64} are allowed
> >>
> >> I think this permission needs to be limited as much as possible.
> >
> > Maybe we should take this out completely given that they should be
> > limited to efi and acpi code that is excepted anyway
>
> I would favor that, yes.
>
> >>> @@ -183,7 +200,10 @@ maintainers if you want to suggest a change.
> >>> * - `Rule 7.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_01.c>`_
> >>> - Required
> >>> - Octal constants shall not be used
> >>> - -
> >>> + - Usage of the following constants is safe, since they are given
> >>> + as-is in the inflate algorithm specification and there is
> >>> + therefore no risk of them being interpreted as decimal constants:
> >>> + ^0(007|37|070|213|236|300|321|330|331|332|333|334|335|337|371)$
> >>
> >> This is a very odd set of exceptions, which by stating them here you then
> >> grant to be exercised everywhere. Once again I don't think special cases
> >> dealing with a single source or sub-component should be globally named.
> >
> > Actually I agree with you there. The problem is where to put them.
>
> safe.json?
Yes you are right
> > Right now we have docs/misra/rules.rst with the list of accepted rules
> > and their special interpretations by Xen Project. We also have the
> > ECLAIR configuration under automation/eclair_analysis with a bunch of
> > ECLAIR specific config files.
> >
> > Is it OK if the constants above only live under
> > automation/eclair_analysis and nowhere else? Or should we have another
> > rst document under docs/misra for special cases dealing with a single
> > source?
>
> As per above, I think putting anything in Eclair's config file should
> only ever be a last resort. Wherever possible we should try to put stuff
> in files which aren't tool specific. Where necessary special tool
> settings can then be (machine-)derived from there.
I agree. I'll send out another patch to update rules.rst. And later as a
follow-up I'll see what I can do to update safe.json.
On 23.08.2023 02:23, Stefano Stabellini wrote: > On Tue, 22 Aug 2023, Jan Beulich wrote: >> On 22.08.2023 03:40, Stefano Stabellini wrote: >>> If we only mention BUG, we are basically saying that as a general rule >>> only BUG is an exception. Then we have a longer more detailed list for >>> ECLAIR because in practice things are always complicated. >>> >>> On the other hand if we have the full list here, then the documentation >>> is more precise, but it looks a bit "strange" to see such a detailed >>> list in this document and also we need to make sure to keep the list >>> up-to-date. >> >> Thing is: This list shouldn't grow very long anyway, and also better >> would grow / change much over time. > > +1 I assume you read what was meant (wouldn't), not what was written (would). Jan
© 2016 - 2024 Red Hat, Inc.