On 17/07/23 15:59, Jan Beulich wrote:
On 14.07.2023 16:20, Luca Fancellu wrote:
On 14 Jul 2023, at 12:49, Nicola Vetrini <nicola.vetr...@bugseng.com> wrote:
The macro 'testop' expands to a function that declares the local
variable 'oldbit', which is written before being set, but is such a
way that is not amenable to automatic checking.
Therefore, a deviation comment, is introduced to document this situation.
A similar reasoning applies to macro 'guest_testop'.
Signed-off-by: Nicola Vetrini <nicola.vetr...@bugseng.com>
---
docs/misra/safe.json | 16 ++++++++++++++++
xen/arch/arm/arm64/lib/bitops.c | 3 +++
xen/arch/arm/include/asm/guest_atomics.h | 3 +++
3 files changed, 22 insertions(+)
diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 244001f5be..4cf7cbf57b 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -20,6 +20,22 @@
},
{
"id": "SAF-2-safe",
+ "analyser": {
+ "eclair": "MC3R1.R9.1"
+ },
+ "name": "Rule 9.1: initializer not needed",
+ "text": "The following local variables are possibly subject to being
read before being written, but code inspection ensured that the control flow in the construct where
they appear ensures that no such event may happen."
+ },
+ {
+ "id": "SAF-3-safe",
+ "analyser": {
+ "eclair": "MC3R1.R9.1"
+ },
+ "name": "Rule 9.1: initializer not needed",
+ "text": "The following local variables are possibly subject to being
read before being written, but code inspection ensured that the control flow in the construct where
they appear ensures that no such event may happen."
+ },
Since the rule and the justification are the same, you can declare only once
and use the same tag on top of the offending lines, so /* SAF-2-safe MC3R1.R9.1
*/,
+1
I'm puzzled by the wording vs comment placement though: The comments
are inserted ahead of the macro invocations, so there are no "following
local variables". Plus does this imply the comment would suppress the
checking on _all_ of them, rather than just the one that was confirmed
to be safe? What if another new one was added, that actually introduces
a problem?
also, I remember some maintainers not happy about the misra rule being put
after the tag, now I don’t recall who
Me, at least. The annotations should be tool-agnostic imo, or else the
more tools we use, the longer these comments might get.
Jan
No problem for both: Given the earlier remarks by Julien on patch 1/4, I
think we can devise something along the lines of
/* SAF-x-safe MISRA:C 2012 Rule 9.1 <Justification> */
int var;
and then write a generic justification in docs/misra/safe.json, while
<Justification> contains a specific one (e.g., this loop does so and so
to ensure that no access to unset variables happens). Thus any
modification to that variable would give who submits the patch and the
reviewer a chance to spot any inconsistency when the code is modified.
However, given my other response to this same patch, this does not apply
here specifically.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
