On 19.08.2023 03:24, Stefano Stabellini wrote: > From: Stefano Stabellini <stefano.stabell...@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
