On 01.07.2024 15:45, Alessandro Zucchelli wrote:
> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
So no description at all for a somewhat unobvious issue with, I think,
a pretty obvious (but entirely different) solution? And that (obvious)
alternative not even being mentioned, together with why it was not
possible to use? Neither ...
> --- a/docs/misra/safe.json
> +++ b/docs/misra/safe.json
> @@ -99,7 +99,15 @@
> "text": "Files intended for multiple inclusion are not supposed to comply with D4.10."
> },
> {
> - "id": "SAF-11-safe",
> + "id": "SAF-12-safe",
> + "analyser": {
> + "eclair": "MC3R1.D4.10"
> + },
> + "name": "Dir 4.10: arch-x86/xen.h include before guard",
> + "text": "This file needs to start with #include ../xen.h to generate preprocessed code in the correct order."
... here nor ...
> + },
> + {
> + "id": "SAF-13-safe",
> "analyser": {},
> "name": "Sentinel",
> "text": "Next ID to be used"
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -7,6 +7,7 @@
> * Copyright (c) 2004-2006, K A Fraser
> */
>
> +/* SAF-12-safe include before guard needed for correct code generation */
> #include "../xen.h"
>
> #ifndef __XEN_PUBLIC_ARCH_X86_XEN_H__
... here is really becomes clear what "correct" is, or what breaks if this
was moved. However, thinking of moving as the first obvious alternative I
checked other arch-specific headers. None includes ../xen.h (really just
xen.h, as the others all live right in public/) like this. Which made me
conclude that maybe there's something wrong with the x86 header doing so.
And indeed, according to my build testing the #include can simple be
dropped, with just one further change elsewhere; see below.
Jan
public/x86: don't include common xen.h from arch-specific one
No other arch-*.h does so, and arch-x86/xen.h really just takes the role
of arch-x86_32.h and arch-x86_64.h (by those two forwarding there). With
xen.h itself including the per-arch headers, doing so is also kind of
backwards anyway, and just calling for problems. There's exactly one
place where arch-x86/xen.h is included when really xen.h is meant (for
wanting XEN_GUEST_HANDLE_64() to be made available, the default
definition of which lives in the common xen.h).
This then addresses a violation of Misra C:2012 Directive 4.10
("Precautions shall be taken in order to prevent the contents of a
header file being included more than once").
Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -7,8 +7,6 @@
* Copyright (c) 2004-2006, K A Fraser
*/
-#include "../xen.h"
-
#ifndef __XEN_PUBLIC_ARCH_X86_XEN_H__
#define __XEN_PUBLIC_ARCH_X86_XEN_H__
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -525,7 +525,7 @@ void x86_cpu_policy_bound_max_leaves(str
void x86_cpu_policy_shrink_max_leaves(struct cpu_policy *p);
#ifdef __XEN__
-#include <public/arch-x86/xen.h>
+#include <public/xen.h>
typedef XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_leaf_buffer_t;
typedef XEN_GUEST_HANDLE_64(xen_msr_entry_t) msr_entry_buffer_t;
#else