| Message ID | 20230609174543.4067574-1-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] docs/misra: new rules addition | expand |
On 09.06.2023 19:45, Stefano Stabellini wrote: > @@ -133,6 +146,13 @@ existing codebase are work-in-progress. > headers (xen/include/public/) are allowed to retain longer > identifiers for backward compatibility. > > + * - `Rule 6.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_01.c>`_ > + - Required > + - Bit-fields shall only be declared with an appropriate type > + - In addition to the C99 types, we also consider appropriate types: > + unsigned char, unsigned short, unsigned long, unsigned long long, > + enum. What about their signed equivalents? I'm surprised that I found only very few uses (in Arm insn decoding afaict), but they generally have a purpose. Are the uses we have (and new ones which may appear) intended to become deviations? > @@ -143,6 +163,12 @@ existing codebase are work-in-progress. > - Octal constants shall not be used > - > > + * - `Rule 7.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_ > + - Required > + - A "u" or "U" suffix shall be applied to all integer constants > + that are represented in an unsigned type > + - I continue to consider "represented in" problematic here without further qualification. > @@ -314,6 +340,11 @@ existing codebase are work-in-progress. > used following a subsequent call to the same function > - > > + * - Rule 21.21 > + - Required > + - The Standard Library function system of <stdlib.h> shall not be used > + - Still no "inapplicable" note (whichever way it would be worded to also please Roberto)? Jan
On 12/06/23 09:33, Jan Beulich wrote: > On 09.06.2023 19:45, Stefano Stabellini wrote: >> @@ -133,6 +146,13 @@ existing codebase are work-in-progress. >> headers (xen/include/public/) are allowed to retain longer >> identifiers for backward compatibility. >> >> + * - `Rule 6.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_01.c>`_ >> + - Required >> + - Bit-fields shall only be declared with an appropriate type >> + - In addition to the C99 types, we also consider appropriate types: >> + unsigned char, unsigned short, unsigned long, unsigned long long, >> + enum. > > What about their signed equivalents? I'm surprised that I found only very > few uses (in Arm insn decoding afaict), but they generally have a purpose. > Are the uses we have (and new ones which may appear) intended to become > deviations? Explicitly signed integer types are all supported by GCC as well. So they can be added to the list. >> @@ -143,6 +163,12 @@ existing codebase are work-in-progress. >> - Octal constants shall not be used >> - >> >> + * - `Rule 7.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_ >> + - Required >> + - A "u" or "U" suffix shall be applied to all integer constants >> + that are represented in an unsigned type >> + - > > I continue to consider "represented in" problematic here without > further qualification. We should distinguish two things here. The headline of Rule 7.2 is non negotiable: it is simply as it is. As all headlines, it is a compromise between conciseness and mnemonic value. If what is wanted there is not the headline, then you can add "implicitly" before "represented". Or you may leave the headline and add an explanatory note afterwards. >> @@ -314,6 +340,11 @@ existing codebase are work-in-progress. >> used following a subsequent call to the same function >> - >> >> + * - Rule 21.21 >> + - Required >> + - The Standard Library function system of <stdlib.h> shall not be used >> + - > > Still no "inapplicable" note (whichever way it would be worded to also > please Roberto)? I am not the one to be pleased ;-) But really, I don't follow: when you say the rule is inapplicable your reasoning is, IIUC, "nobody would even dream using system() in Xen". Which is exactly what the rule is asking. If Xen adopts the rule, tooling will make sure system() is not used, and seeing that the rule is applied, assessors will be pleased.
On 12.06.2023 11:34, Roberto Bagnara wrote: > On 12/06/23 09:33, Jan Beulich wrote: >> On 09.06.2023 19:45, Stefano Stabellini wrote: >>> @@ -143,6 +163,12 @@ existing codebase are work-in-progress. >>> - Octal constants shall not be used >>> - >>> >>> + * - `Rule 7.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_ >>> + - Required >>> + - A "u" or "U" suffix shall be applied to all integer constants >>> + that are represented in an unsigned type >>> + - >> >> I continue to consider "represented in" problematic here without >> further qualification. > > We should distinguish two things here. The headline of Rule 7.2 > is non negotiable: it is simply as it is. I understand this, and ... > As all headlines, > it is a compromise between conciseness and mnemonic value. > If what is wanted there is not the headline, then you can add > "implicitly" before "represented". Or you may leave the headline > and add an explanatory note afterwards. ... such a note is what my comment was heading towards. >>> @@ -314,6 +340,11 @@ existing codebase are work-in-progress. >>> used following a subsequent call to the same function >>> - >>> >>> + * - Rule 21.21 >>> + - Required >>> + - The Standard Library function system of <stdlib.h> shall not be used >>> + - >> >> Still no "inapplicable" note (whichever way it would be worded to also >> please Roberto)? > > I am not the one to be pleased ;-) > > But really, I don't follow: when you say the rule is inapplicable > your reasoning is, IIUC, "nobody would even dream using system() in Xen". > Which is exactly what the rule is asking. If Xen adopts the rule, > tooling will make sure system() is not used, and seeing that the rule > is applied, assessors will be pleased. My point is that "not using functions of stdlib.h" is ambiguous: It may mean functions implemented in an external library (which the hypervisor doesn't use), or it may mean functions of identical name (and purpose). The full text goes even further and forbids the use of these identifiers (plural; see next paragraph), so it's clearly not only about an external library, and we also can't put it off as inapplicable. (I wouldn't be surprised if we had a local variable or label named "exit" or "abort".) Btw - I can't find a rule 21.21 in my two (slightly different) copies of the doc, nor one with this headline and a different number. What I have is "21.8 The Standard Library functions abort, exit and system of <stdlib.h> shall not be used". (I further wonder why neither of the two docs allows me to copy-and-paste a line out of it.) Jan
On 12/06/23 11:50, Jan Beulich wrote: > On 12.06.2023 11:34, Roberto Bagnara wrote: >> On 12/06/23 09:33, Jan Beulich wrote: >>> On 09.06.2023 19:45, Stefano Stabellini wrote: >>>> @@ -143,6 +163,12 @@ existing codebase are work-in-progress. >>>> - Octal constants shall not be used >>>> - >>>> >>>> + * - `Rule 7.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_ >>>> + - Required >>>> + - A "u" or "U" suffix shall be applied to all integer constants >>>> + that are represented in an unsigned type >>>> + - >>> >>> I continue to consider "represented in" problematic here without >>> further qualification. >> >> We should distinguish two things here. The headline of Rule 7.2 >> is non negotiable: it is simply as it is. > > I understand this, and ... > >> As all headlines, >> it is a compromise between conciseness and mnemonic value. >> If what is wanted there is not the headline, then you can add >> "implicitly" before "represented". Or you may leave the headline >> and add an explanatory note afterwards. > > ... such a note is what my comment was heading towards. Here is an attempt. "The rule asks that any integer literal that is implicitly unsigned is made explicitly unsigned by using one of the indicated suffixes. As an example, on a machine where the int type is 32-bit wide, 0x77777777 is signed whereas 0x80000000 is (implicitly) unsigned. In order to comply with the rule, the latter should be rewritten as either 0x80000000u or 0x80000000U. Consistency considerations may suggest using the same suffix even when not required by the rule. For instance, if one has f(0x77777777); // Original f(0x80000000); one might prefer f(0x77777777U); // Solution 1 f(0x80000000U); over f(0x77777777); // Solution 2 f(0x80000000U); after having ascertained that "Solution 1" is compatible with the intended semantics." >>>> @@ -314,6 +340,11 @@ existing codebase are work-in-progress. >>>> used following a subsequent call to the same function >>>> - >>>> >>>> + * - Rule 21.21 >>>> + - Required >>>> + - The Standard Library function system of <stdlib.h> shall not be used >>>> + - >>> >>> Still no "inapplicable" note (whichever way it would be worded to also >>> please Roberto)? >> >> I am not the one to be pleased ;-) >> >> But really, I don't follow: when you say the rule is inapplicable >> your reasoning is, IIUC, "nobody would even dream using system() in Xen". >> Which is exactly what the rule is asking. If Xen adopts the rule, >> tooling will make sure system() is not used, and seeing that the rule >> is applied, assessors will be pleased. > > My point is that "not using functions of stdlib.h" is ambiguous: It may > mean functions implemented in an external library (which the hypervisor > doesn't use), or it may mean functions of identical name (and purpose). > The full text goes even further and forbids the use of these > identifiers (plural; see next paragraph), so it's clearly not only > about an external library, and we also can't put it off as inapplicable. > (I wouldn't be surprised if we had a local variable or label named > "exit" or "abort".) > > Btw - I can't find a rule 21.21 in my two (slightly different) copies > of the doc, nor one with this headline and a different number. What I > have is "21.8 The Standard Library functions abort, exit and system of > <stdlib.h> shall not be used". (I further wonder why neither of the two > docs allows me to copy-and-paste a line out of it.) Rule 21.21 was added in MISRA C:2012 Amendment 2, which you can download (free of charge) from https://www.misra.org.uk/app/uploads/2021/06/MISRA-C-2012-AMD2.pdf
diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst index d5a6ee8cb6..8be6868496 100644 --- a/docs/misra/rules.rst +++ b/docs/misra/rules.rst @@ -40,6 +40,12 @@ existing codebase are work-in-progress. - Summary - Notes + * - `Dir 1.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_01_01.c>`_ + - Required + - Any implementation-defined behaviour on which the output of the + program depends shall be documented and understood + - + * - `Dir 2.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_02_01.c>`_ - Required - All source files shall compile without any compilation errors @@ -57,6 +63,13 @@ existing codebase are work-in-progress. header file being included more than once - + * - `Dir 4.11 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_11.c>`_ + - Required + - The validity of values passed to library functions shall be checked + - We do not have libraries in Xen (libfdt and others are not + considered libraries from MISRA C point of view as they are + imported in source form) + * - `Dir 4.14 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_14.c>`_ - Required - The validity of values received from external sources shall be @@ -133,6 +146,13 @@ existing codebase are work-in-progress. headers (xen/include/public/) are allowed to retain longer identifiers for backward compatibility. + * - `Rule 6.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_01.c>`_ + - Required + - Bit-fields shall only be declared with an appropriate type + - In addition to the C99 types, we also consider appropriate types: + unsigned char, unsigned short, unsigned long, unsigned long long, + enum. + * - `Rule 6.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_02.c>`_ - Required - Single-bit named bit fields shall not be of a signed type @@ -143,6 +163,12 @@ existing codebase are work-in-progress. - Octal constants shall not be used - + * - `Rule 7.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_ + - Required + - A "u" or "U" suffix shall be applied to all integer constants + that are represented in an unsigned type + - + * - `Rule 7.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_03.c>`_ - Required - The lowercase character l shall not be used in a literal suffix @@ -314,6 +340,11 @@ existing codebase are work-in-progress. used following a subsequent call to the same function - + * - Rule 21.21 + - Required + - The Standard Library function system of <stdlib.h> shall not be used + - + * - `Rule 22.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_22_02.c>`_ - Mandatory - A block of memory shall only be freed if it was allocated by means of a