On 17/07/23 17:46, Jan Beulich wrote:
On 17.07.2023 17:28, Nicola Vetrini wrote:
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.
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).
But that still mentions the rule. What if a new MISRA:C 2025 appears,
with different rule numbers? I don't mind the comment mentioning, in
a sufficiently terse way, what problem is circumvented. But I don't
think tools or specifications should be referenced here. That's the
purpose of the referenced "database" entry.
Jan
Ok
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
