[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH] x86/vlapic: address a violation of MISRA C:2012 Rule 16.2



On 2023-11-21 16:36, Jan Beulich wrote:
On 25.10.2023 15:22, Nicola Vetrini wrote:
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1034,10 +1034,10 @@ int guest_wrmsr_x2apic(struct vcpu *v, uint32_t msr, uint64_t val)
     case APIC_EOI:
     case APIC_ESR:
         if ( val )
-        {
-    default:
             return X86EMUL_EXCEPTION;
-        }
+        break;
+    default:
+        return X86EMUL_EXCEPTION;
     }

     vlapic_reg_write(v, array_index_nospec(offset, PAGE_SIZE), val);

Considering the plan to confine applicability of the rule, one style aspect which would need to be taken into account is that the entire rest of this
switch() has blank lines between case blocks.

The other is that imo the overall result would be closer to what we have
right now if the new code was

    case APIC_EOI:
    case APIC_ESR:
        if ( !val )
            break;
        fallthrough;
    default:
        return X86EMUL_EXCEPTION;
     }

at which point the need for the blank line would also disappear.


This is also a fine solution. I'll keep this in mind when this patch will be revisited.

As to the description - isn't this change (whichever way done) also
addressing another violation, requiring "break" (or alike according to
our interpretation) at the end of each case block?


Correct. It's probably a good idea to mention that, but the "fallthrough" is also a candidate for a deviation from R16.3, so we'll see about that.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.