| Message ID | 9be1812e391ce893afa028c05614b74cddb197b7.1730880832.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: address violations of MISRA C Rule 16.3 | expand |
On 06.11.2024 10:04, Federico Serafini wrote: > Make explicit the fallthrough intetion by adding the pseudo keyword > where missing and refactor comments not following the agreed syntax. > > This satisfies the requirements to deviate violations of > MISRA C:2012 Rule 16.3 "An unconditional break statement shall > terminate every switch-clause". > > No functional change. > > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> We can certainly take this as is, as a tiny first step. Personally I'm not overly happy though to see a mix of comment- and pseudo-keyword- style annotations in individual files. After all going forward we'll want to use the latter, now that this becomes possible. Yet for that I view it a more than just helpful if bad precedents didn't exist anymore. Being the one to touch the emulator files most, I can say that here more than perhaps anywhere else new code is very often added by copy-and-paste-then-edit. Therefore question in particular to the other x86 maintainers: Won't we be better off if we fully convert to pseudo-keyword-style right away? Jan > --- a/xen/arch/x86/x86_emulate/decode.c > +++ b/xen/arch/x86/x86_emulate/decode.c > @@ -1356,7 +1356,8 @@ int x86emul_decode(struct x86_emulate_state *s, > --disp8scale; > break; > } > - /* vcvt{,t}s{s,d}2usi need special casing: fall through */ > + /* vcvt{,t}s{s,d}2usi need special casing. */ > + fallthrough; > case 0x2c: /* vcvtts{s,d}2si need special casing */ > case 0x2d: /* vcvts{s,d}2si need special casing */ > if ( evex_encoded() ) > @@ -1530,7 +1531,8 @@ int x86emul_decode(struct x86_emulate_state *s, > disp8scale -= 1 + (s->evex.pfx == vex_66); > break; > } > - /* vcvt{,t}sh2usi needs special casing: fall through */ > + /* vcvt{,t}sh2usi needs special casing. */ > + fallthrough; > case 0x2c: case 0x2d: /* vcvt{,t}sh2si need special casing */ > disp8scale = 1; > break; > --- a/xen/arch/x86/x86_emulate/x86_emulate.c > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c > @@ -1460,6 +1460,7 @@ x86_emulate( > > d = (d & ~DstMask) | DstMem; > /* Becomes a normal DstMem operation from here on. */ > + fallthrough; > case DstMem: > generate_exception_if(ea.type == OP_MEM && evex.z, X86_EXC_UD); > if ( state->simd_size != simd_none ) > @@ -1942,6 +1943,7 @@ x86_emulate( > break; > } > generate_exception_if((modrm_reg & 7) != 0, X86_EXC_UD); > + fallthrough; > case 0x88 ... 0x8b: /* mov */ > case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */ > case 0xa2 ... 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */
On Wed, 6 Nov 2024, Federico Serafini wrote: > Make explicit the fallthrough intetion by adding the pseudo keyword > where missing and refactor comments not following the agreed syntax. > > This satisfies the requirements to deviate violations of > MISRA C:2012 Rule 16.3 "An unconditional break statement shall > terminate every switch-clause". > > No functional change. > > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/x86/x86_emulate/decode.c | 6 ++++-- > xen/arch/x86/x86_emulate/x86_emulate.c | 2 ++ > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/x86_emulate/decode.c b/xen/arch/x86/x86_emulate/decode.c > index 32b9276dc5..0a0751f2ed 100644 > --- a/xen/arch/x86/x86_emulate/decode.c > +++ b/xen/arch/x86/x86_emulate/decode.c > @@ -1356,7 +1356,8 @@ int x86emul_decode(struct x86_emulate_state *s, > --disp8scale; > break; > } > - /* vcvt{,t}s{s,d}2usi need special casing: fall through */ > + /* vcvt{,t}s{s,d}2usi need special casing. */ > + fallthrough; > case 0x2c: /* vcvtts{s,d}2si need special casing */ > case 0x2d: /* vcvts{s,d}2si need special casing */ > if ( evex_encoded() ) > @@ -1530,7 +1531,8 @@ int x86emul_decode(struct x86_emulate_state *s, > disp8scale -= 1 + (s->evex.pfx == vex_66); > break; > } > - /* vcvt{,t}sh2usi needs special casing: fall through */ > + /* vcvt{,t}sh2usi needs special casing. */ > + fallthrough; > case 0x2c: case 0x2d: /* vcvt{,t}sh2si need special casing */ > disp8scale = 1; > break; > diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c > index 30674ec301..c38984b201 100644 > --- a/xen/arch/x86/x86_emulate/x86_emulate.c > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c > @@ -1460,6 +1460,7 @@ x86_emulate( > > d = (d & ~DstMask) | DstMem; > /* Becomes a normal DstMem operation from here on. */ > + fallthrough; > case DstMem: > generate_exception_if(ea.type == OP_MEM && evex.z, X86_EXC_UD); > if ( state->simd_size != simd_none ) > @@ -1942,6 +1943,7 @@ x86_emulate( > break; > } > generate_exception_if((modrm_reg & 7) != 0, X86_EXC_UD); > + fallthrough; > case 0x88 ... 0x8b: /* mov */ > case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */ > case 0xa2 ... 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */ > -- > 2.43.0 >
diff --git a/xen/arch/x86/x86_emulate/decode.c b/xen/arch/x86/x86_emulate/decode.c index 32b9276dc5..0a0751f2ed 100644 --- a/xen/arch/x86/x86_emulate/decode.c +++ b/xen/arch/x86/x86_emulate/decode.c @@ -1356,7 +1356,8 @@ int x86emul_decode(struct x86_emulate_state *s, --disp8scale; break; } - /* vcvt{,t}s{s,d}2usi need special casing: fall through */ + /* vcvt{,t}s{s,d}2usi need special casing. */ + fallthrough; case 0x2c: /* vcvtts{s,d}2si need special casing */ case 0x2d: /* vcvts{s,d}2si need special casing */ if ( evex_encoded() ) @@ -1530,7 +1531,8 @@ int x86emul_decode(struct x86_emulate_state *s, disp8scale -= 1 + (s->evex.pfx == vex_66); break; } - /* vcvt{,t}sh2usi needs special casing: fall through */ + /* vcvt{,t}sh2usi needs special casing. */ + fallthrough; case 0x2c: case 0x2d: /* vcvt{,t}sh2si need special casing */ disp8scale = 1; break; diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c index 30674ec301..c38984b201 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -1460,6 +1460,7 @@ x86_emulate( d = (d & ~DstMask) | DstMem; /* Becomes a normal DstMem operation from here on. */ + fallthrough; case DstMem: generate_exception_if(ea.type == OP_MEM && evex.z, X86_EXC_UD); if ( state->simd_size != simd_none ) @@ -1942,6 +1943,7 @@ x86_emulate( break; } generate_exception_if((modrm_reg & 7) != 0, X86_EXC_UD); + fallthrough; case 0x88 ... 0x8b: /* mov */ case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */ case 0xa2 ... 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */
Make explicit the fallthrough intetion by adding the pseudo keyword where missing and refactor comments not following the agreed syntax. This satisfies the requirements to deviate violations of MISRA C:2012 Rule 16.3 "An unconditional break statement shall terminate every switch-clause". No functional change. Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> --- xen/arch/x86/x86_emulate/decode.c | 6 ++++-- xen/arch/x86/x86_emulate/x86_emulate.c | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-)