Refactor the switch so that a violation of MISRA C Rule 16.2 is resolved
(A switch label shall only be used when the most closely-enclosing
compound statement is the body of a switch statement).
Note that the switch clause ending with the pseudo
keyword "fallthrough" is an allowed exception to Rule 16.3.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
xen/arch/x86/cpuid.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 7290a979c667..0a7c55199f94 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -331,23 +331,22 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
switch ( subleaf )
{
case 1:
- if ( p->xstate.xsavec || p->xstate.xsaves )
- {
- /*
- * TODO: Figure out what to do for XSS state. VT-x manages
- * host vs guest MSR_XSS automatically, so as soon as we start
- * supporting any XSS states, the wrong XSS will be in
- * context.
- */
- BUILD_BUG_ON(XSTATE_XSAVES_ONLY != 0);
-
- /*
- * Read CPUID[0xD,0/1].EBX from hardware. They vary with
- * enabled XSTATE, and appropraite XCR0|XSS are in context.
- */
+ if ( !(p->xstate.xsavec || p->xstate.xsaves) )
+ break;
+ /*
+ * TODO: Figure out what to do for XSS state. VT-x manages
+ * host vs guest MSR_XSS automatically, so as soon as we start
+ * supporting any XSS states, the wrong XSS will be in
+ * context.
+ */
+ BUILD_BUG_ON(XSTATE_XSAVES_ONLY != 0);
+ fallthrough;
case 0:
- res->b = cpuid_count_ebx(leaf, subleaf);
- }
+ /*
+ * Read CPUID[0xD,0/1].EBX from hardware. They vary with
+ * enabled XSTATE, and appropriate XCR0|XSS are in context.
+ */
+ res->b = cpuid_count_ebx(leaf, subleaf);
break;
}
break;
--
2.34.1On 05.04.2024 11:14, Nicola Vetrini wrote:
> Refactor the switch so that a violation of MISRA C Rule 16.2 is resolved
> (A switch label shall only be used when the most closely-enclosing
> compound statement is the body of a switch statement).
> Note that the switch clause ending with the pseudo
> keyword "fallthrough" is an allowed exception to Rule 16.3.
>
> No functional change.
>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
albeit once again with remarks:
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -331,23 +331,22 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
> switch ( subleaf )
> {
> case 1:
> - if ( p->xstate.xsavec || p->xstate.xsaves )
> - {
> - /*
> - * TODO: Figure out what to do for XSS state. VT-x manages
> - * host vs guest MSR_XSS automatically, so as soon as we start
> - * supporting any XSS states, the wrong XSS will be in
> - * context.
> - */
> - BUILD_BUG_ON(XSTATE_XSAVES_ONLY != 0);
> -
> - /*
> - * Read CPUID[0xD,0/1].EBX from hardware. They vary with
> - * enabled XSTATE, and appropraite XCR0|XSS are in context.
> - */
> + if ( !(p->xstate.xsavec || p->xstate.xsaves) )
Personally I dislike with for of writing such. It may not be overly much of a
problem for simple cases like here, but the more complex the expression gets,
the less helpful it is that somewhere far away there's an enclosing !(...). I
may take the liberty to adjust this, should I end up committing this change.
> + break;
> + /*
> + * TODO: Figure out what to do for XSS state. VT-x manages
> + * host vs guest MSR_XSS automatically, so as soon as we start
> + * supporting any XSS states, the wrong XSS will be in
> + * context.
> + */
Much like one actually needs to consider re-flowing when increasing indentation
of a comment, it is generally desirable to also to so when decreasing
indentation, which in this case surely would allow at least "context" to be
moved to the earlier line.
Jan
On 2024-04-08 09:39, Jan Beulich wrote:
> On 05.04.2024 11:14, Nicola Vetrini wrote:
>> Refactor the switch so that a violation of MISRA C Rule 16.2 is
>> resolved
>> (A switch label shall only be used when the most closely-enclosing
>> compound statement is the body of a switch statement).
>> Note that the switch clause ending with the pseudo
>> keyword "fallthrough" is an allowed exception to Rule 16.3.
>>
>> No functional change.
>>
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> albeit once again with remarks:
>
>> --- a/xen/arch/x86/cpuid.c
>> +++ b/xen/arch/x86/cpuid.c
>> @@ -331,23 +331,22 @@ void guest_cpuid(const struct vcpu *v, uint32_t
>> leaf,
>> switch ( subleaf )
>> {
>> case 1:
>> - if ( p->xstate.xsavec || p->xstate.xsaves )
>> - {
>> - /*
>> - * TODO: Figure out what to do for XSS state. VT-x
>> manages
>> - * host vs guest MSR_XSS automatically, so as soon as
>> we start
>> - * supporting any XSS states, the wrong XSS will be
>> in
>> - * context.
>> - */
>> - BUILD_BUG_ON(XSTATE_XSAVES_ONLY != 0);
>> -
>> - /*
>> - * Read CPUID[0xD,0/1].EBX from hardware. They vary
>> with
>> - * enabled XSTATE, and appropraite XCR0|XSS are in
>> context.
>> - */
>> + if ( !(p->xstate.xsavec || p->xstate.xsaves) )
>
> Personally I dislike with for of writing such. It may not be overly
> much of a
> problem for simple cases like here, but the more complex the expression
> gets,
> the less helpful it is that somewhere far away there's an enclosing
> !(...). I
> may take the liberty to adjust this, should I end up committing this
> change.
>
Ok, makes sense. I didn't think about that style aspect.
>> + break;
>> + /*
>> + * TODO: Figure out what to do for XSS state. VT-x
>> manages
>> + * host vs guest MSR_XSS automatically, so as soon as we
>> start
>> + * supporting any XSS states, the wrong XSS will be in
>> + * context.
>> + */
>
> Much like one actually needs to consider re-flowing when increasing
> indentation
> of a comment, it is generally desirable to also to so when decreasing
> indentation, which in this case surely would allow at least "context"
> to be
> moved to the earlier line.
>
> Jan
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
© 2016 - 2026 Red Hat, Inc.