diff mbox series

[XEN,3/4] x86: address violations of MISRA C Rule 8.4

Message ID cf926194a541d11e02670516a8d337de27836dce.1716814609.git.nicola.vetrini@bugseng.com (mailing list archive)
State New, archived
Headers show
Series various ECLAIR and MISRA improvements | expand

Commit Message

Nicola Vetrini May 27, 2024, 2:53 p.m. UTC
Rule 8.4 states: "A compatible declaration shall be visible when
an object or function with external linkage is defined."

These variables are only referenced from asm modules, so they
need to be extern and there is negligible risk of them being
used improperly without noticing.

As a result, they can be exempted using a comment-based deviation.
No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Adding the asmlinkage macro to variables is not appropriate, as
this pseudo-attribute may expand, for instance, to a different calling
convention in the future (e.g. stdcall)
---
 xen/arch/x86/desc.c | 1 +
 xen/arch/x86/mm.c   | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

Jan Beulich May 28, 2024, 6:28 a.m. UTC | #1
On 27.05.2024 16:53, Nicola Vetrini wrote:
> Rule 8.4 states: "A compatible declaration shall be visible when
> an object or function with external linkage is defined."
> 
> These variables are only referenced from asm modules, so they
> need to be extern and there is negligible risk of them being
> used improperly without noticing.

"asm modules" isn't quite correct, as there's one use from inline
assembly. I have to admit I have no good wording suggestion other than
explicitly covering both: "asm modules or inline assembly". Yet that
then is ambiguous, as a use in inline assembly may also mean that
symbol is actually visible to the compiler by being mentioned as on of
the operands. Better ideas?

> As a result, they can be exempted using a comment-based deviation.
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

With suitably adjusted wording:
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan
Nicola Vetrini May 28, 2024, 8:39 p.m. UTC | #2
On 2024-05-28 08:28, Jan Beulich wrote:
> On 27.05.2024 16:53, Nicola Vetrini wrote:
>> Rule 8.4 states: "A compatible declaration shall be visible when
>> an object or function with external linkage is defined."
>> 
>> These variables are only referenced from asm modules, so they
>> need to be extern and there is negligible risk of them being
>> used improperly without noticing.
> 
> "asm modules" isn't quite correct, as there's one use from inline
> assembly. I have to admit I have no good wording suggestion other than
> explicitly covering both: "asm modules or inline assembly". Yet that
> then is ambiguous, as a use in inline assembly may also mean that
> symbol is actually visible to the compiler by being mentioned as on of
> the operands. Better ideas?
> 

Maybe generically "asm code" or just "asm"? It's not really relevant 
whether it's inline or not, as far as I understand.

>> As a result, they can be exempted using a comment-based deviation.
>> No functional change.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> With suitably adjusted wording:
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/desc.c b/xen/arch/x86/desc.c
index 39080ca67211..9f639281540a 100644
--- a/xen/arch/x86/desc.c
+++ b/xen/arch/x86/desc.c
@@ -91,6 +91,7 @@  seg_desc_t boot_compat_gdt[PAGE_SIZE / sizeof(seg_desc_t)] =
  * References boot_cpu_gdt_table for a short period, until the CPUs switch
  * onto their per-CPU GDTs.
  */
+/* SAF-1-safe */
 const struct desc_ptr boot_gdtr = {
     .limit = LAST_RESERVED_GDT_BYTE,
     .base = (unsigned long)(boot_gdt - FIRST_RESERVED_GDT_ENTRY),
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index d968bbbc7315..17987eb5199e 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -144,7 +144,7 @@ 
 l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE)
     l1_fixmap[L1_PAGETABLE_ENTRIES];
 l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE)
-    l1_fixmap_x[L1_PAGETABLE_ENTRIES];
+    l1_fixmap_x[L1_PAGETABLE_ENTRIES]; /* SAF-1-safe */
 
 bool __read_mostly machine_to_phys_mapping_valid;