| Message ID | f957c92d9a00c66df47fc3cac336e378488b9fea.1712305581.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | address violations of MISRA C Rule 16.2 | expand |
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. > + 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
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;
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(-)