Variable declarations between a switch statement guard and before
any case label are unreachable code, and hence violate Rule 2.1:
"A project shall not contain unreachable code".
Therefore the variable declarations are moved in the only clause
scope that uses it.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
xen/arch/x86/hvm/emulate.c | 9 ++++-----
xen/arch/x86/hvm/hvm.c | 10 ++++------
xen/arch/x86/x86_emulate/0f01.c | 7 +++----
xen/arch/x86/x86_emulate/blk.c | 17 ++++++++---------
xen/arch/x86/x86_emulate/decode.c | 3 ++-
xen/arch/x86/x86_emulate/fpu.c | 3 +--
xen/arch/x86/x86_emulate/util-xen.c | 4 ++--
xen/arch/x86/x86_emulate/x86_emulate.c | 14 +++++++-------
8 files changed, 31 insertions(+), 36 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 75ee98a73b..2261e8655b 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1994,6 +1994,7 @@ static int cf_check hvmemul_rep_stos(
bool_t df = !!(ctxt->regs->eflags & X86_EFLAGS_DF);
int rc = hvmemul_virtual_to_linear(seg, offset, bytes_per_rep, reps,
hvm_access_write, hvmemul_ctxt, &addr);
+ char *buf;
if ( rc != X86EMUL_OKAY )
return rc;
@@ -2025,7 +2026,6 @@ static int cf_check hvmemul_rep_stos(
switch ( p2mt )
{
unsigned long bytes;
- char *buf;
default:
/* Allocate temporary buffer. */
@@ -2289,16 +2289,15 @@ static int cf_check hvmemul_cache_op(
struct hvm_emulate_ctxt *hvmemul_ctxt =
container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
uint32_t pfec = PFEC_page_present;
+ unsigned long addr;
+ int rc;
+ void *mapping;
if ( !cache_flush_permitted(current->domain) )
return X86EMUL_OKAY;
switch ( op )
{
- unsigned long addr;
- int rc;
- void *mapping;
-
case x86emul_clflush:
case x86emul_clflushopt:
case x86emul_clwb:
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 2180abeb33..4170343b34 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1494,10 +1494,10 @@ static int cf_check hvm_load_cpu_msrs(struct domain *d, hvm_domain_context_t *h)
for ( i = 0; i < ctxt->count; ++i )
{
+ int rc;
+
switch ( ctxt->msr[i].index )
{
- int rc;
-
case MSR_SPEC_CTRL:
case MSR_INTEL_MISC_FEATURES_ENABLES:
case MSR_PKRS:
@@ -3522,6 +3522,7 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
struct domain *d = v->domain;
uint64_t *var_range_base, *fixed_range_base;
int ret;
+ unsigned int index;
var_range_base = (uint64_t *)v->arch.hvm.mtrr.var_ranges;
fixed_range_base = (uint64_t *)v->arch.hvm.mtrr.fixed_ranges;
@@ -3533,8 +3534,6 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
switch ( msr )
{
- unsigned int index;
-
case MSR_EFER:
*msr_content = v->arch.hvm.guest_efer;
break;
@@ -3636,6 +3635,7 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
struct vcpu *v = current;
struct domain *d = v->domain;
int ret;
+ unsigned int index;
HVMTRACE_3D(MSR_WRITE, msr,
(uint32_t)msr_content, (uint32_t)(msr_content >> 32));
@@ -3668,8 +3668,6 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
switch ( msr )
{
- unsigned int index;
-
case MSR_EFER:
if ( hvm_set_efer(msr_content) )
return X86EMUL_EXCEPTION;
diff --git a/xen/arch/x86/x86_emulate/0f01.c b/xen/arch/x86/x86_emulate/0f01.c
index ba43fc394b..22a14b12c3 100644
--- a/xen/arch/x86/x86_emulate/0f01.c
+++ b/xen/arch/x86/x86_emulate/0f01.c
@@ -24,13 +24,12 @@ int x86emul_0f01(struct x86_emulate_state *s,
{
enum x86_segment seg = (s->modrm_reg & 1) ? x86_seg_idtr : x86_seg_gdtr;
int rc;
+ unsigned long base, limit, cr0, cr0w, cr4;
+ struct segment_register sreg;
+ uint64_t msr_val;
switch ( s->modrm )
{
- unsigned long base, limit, cr0, cr0w, cr4;
- struct segment_register sreg;
- uint64_t msr_val;
-
case 0xc6:
switch ( s->vex.pfx )
{
diff --git a/xen/arch/x86/x86_emulate/blk.c b/xen/arch/x86/x86_emulate/blk.c
index e790f4f900..f2956c0d52 100644
--- a/xen/arch/x86/x86_emulate/blk.c
+++ b/xen/arch/x86/x86_emulate/blk.c
@@ -26,19 +26,18 @@ int x86_emul_blk(
struct x86_emulate_ctxt *ctxt)
{
int rc = X86EMUL_OKAY;
-
- switch ( s->blk )
- {
- bool zf;
+ bool zf;
#ifndef X86EMUL_NO_FPU
+ struct {
+ struct x87_env32 env;
struct {
- struct x87_env32 env;
- struct {
- uint8_t bytes[10];
- } freg[8];
- } fpstate;
+ uint8_t bytes[10];
+ } freg[8];
+ } fpstate;
#endif
+ switch ( s->blk )
+ {
/*
* Throughout this switch(), memory clobbers are used to compensate
* that other operands may not properly express the (full) memory
diff --git a/xen/arch/x86/x86_emulate/decode.c b/xen/arch/x86/x86_emulate/decode.c
index f58ca3984e..daebf3a9bd 100644
--- a/xen/arch/x86/x86_emulate/decode.c
+++ b/xen/arch/x86/x86_emulate/decode.c
@@ -1758,9 +1758,9 @@ int x86emul_decode(struct x86_emulate_state *s,
/* Fetch the immediate operand, if present. */
switch ( d & SrcMask )
{
+ case SrcImm: {
unsigned int bytes;
- case SrcImm:
if ( !(d & ByteOp) )
{
if ( mode_64bit() && !amd_like(ctxt) &&
@@ -1785,6 +1785,7 @@ int x86emul_decode(struct x86_emulate_state *s,
case SrcImm16:
s->imm1 = insn_fetch_type(uint16_t);
break;
+ }
}
ctxt->opcode = opcode;
diff --git a/xen/arch/x86/x86_emulate/fpu.c b/xen/arch/x86/x86_emulate/fpu.c
index 480d879657..002d5e1253 100644
--- a/xen/arch/x86/x86_emulate/fpu.c
+++ b/xen/arch/x86/x86_emulate/fpu.c
@@ -84,11 +84,10 @@ int x86emul_fpu(struct x86_emulate_state *s,
uint8_t b;
int rc;
struct x86_emulate_stub stub = {};
+ unsigned long dummy;
switch ( b = ctxt->opcode )
{
- unsigned long dummy;
-
case 0x9b: /* wait/fwait */
host_and_vcpu_must_have(fpu);
get_fpu(X86EMUL_FPU_wait);
diff --git a/xen/arch/x86/x86_emulate/util-xen.c b/xen/arch/x86/x86_emulate/util-xen.c
index 5e90818010..7ab2ac712f 100644
--- a/xen/arch/x86/x86_emulate/util-xen.c
+++ b/xen/arch/x86/x86_emulate/util-xen.c
@@ -77,10 +77,10 @@ bool cf_check x86_insn_is_portio(const struct x86_emulate_state *s,
bool cf_check x86_insn_is_cr_access(const struct x86_emulate_state *s,
const struct x86_emulate_ctxt *ctxt)
{
+ unsigned int ext;
+
switch ( ctxt->opcode )
{
- unsigned int ext;
-
case X86EMUL_OPC(0x0f, 0x01):
if ( x86_insn_modrm(s, NULL, &ext) >= 0
&& (ext & 5) == 4 ) /* SMSW / LMSW */
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index e88245eae9..d6c04fd5f3 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1479,15 +1479,15 @@ x86_emulate(
break;
}
+ enum x86_segment seg;
+ struct segment_register cs, sreg;
+ struct cpuid_leaf leaf;
+ uint64_t msr_val;
+ unsigned int i, n;
+ unsigned long dummy;
+
switch ( ctxt->opcode )
{
- enum x86_segment seg;
- struct segment_register cs, sreg;
- struct cpuid_leaf leaf;
- uint64_t msr_val;
- unsigned int i, n;
- unsigned long dummy;
-
case 0x00: case 0x01: add: /* add reg,mem */
if ( ops->rmw && dst.type == OP_MEM )
state->rmw = rmw_add;
--
2.34.1On Wed, 2 Aug 2023, Nicola Vetrini wrote:
> Variable declarations between a switch statement guard and before
> any case label are unreachable code, and hence violate Rule 2.1:
> "A project shall not contain unreachable code".
>
> Therefore the variable declarations are moved in the only clause
> scope that uses it.
>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
> xen/arch/x86/hvm/emulate.c | 9 ++++-----
> xen/arch/x86/hvm/hvm.c | 10 ++++------
> xen/arch/x86/x86_emulate/0f01.c | 7 +++----
> xen/arch/x86/x86_emulate/blk.c | 17 ++++++++---------
> xen/arch/x86/x86_emulate/decode.c | 3 ++-
> xen/arch/x86/x86_emulate/fpu.c | 3 +--
> xen/arch/x86/x86_emulate/util-xen.c | 4 ++--
> xen/arch/x86/x86_emulate/x86_emulate.c | 14 +++++++-------
> 8 files changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 75ee98a73b..2261e8655b 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1994,6 +1994,7 @@ static int cf_check hvmemul_rep_stos(
> bool_t df = !!(ctxt->regs->eflags & X86_EFLAGS_DF);
> int rc = hvmemul_virtual_to_linear(seg, offset, bytes_per_rep, reps,
> hvm_access_write, hvmemul_ctxt, &addr);
> + char *buf;
>
> if ( rc != X86EMUL_OKAY )
> return rc;
> @@ -2025,7 +2026,6 @@ static int cf_check hvmemul_rep_stos(
> switch ( p2mt )
> {
> unsigned long bytes;
> - char *buf;
why can "bytes" where it is?
Bith buf and bytes could be declared under "default" with a new block
> default:
> /* Allocate temporary buffer. */
> @@ -2289,16 +2289,15 @@ static int cf_check hvmemul_cache_op(
> struct hvm_emulate_ctxt *hvmemul_ctxt =
> container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> uint32_t pfec = PFEC_page_present;
> + unsigned long addr;
> + int rc;
> + void *mapping;
>
> if ( !cache_flush_permitted(current->domain) )
> return X86EMUL_OKAY;
>
> switch ( op )
> {
> - unsigned long addr;
> - int rc;
> - void *mapping;
These three could be...
> case x86emul_clflush:
> case x86emul_clflushopt:
> case x86emul_clwb:
... here in a new block
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 2180abeb33..4170343b34 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1494,10 +1494,10 @@ static int cf_check hvm_load_cpu_msrs(struct domain *d, hvm_domain_context_t *h)
>
> for ( i = 0; i < ctxt->count; ++i )
> {
> + int rc;
> +
> switch ( ctxt->msr[i].index )
> {
> - int rc;
> -
> case MSR_SPEC_CTRL:
> case MSR_INTEL_MISC_FEATURES_ENABLES:
> case MSR_PKRS:
> @@ -3522,6 +3522,7 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
> struct domain *d = v->domain;
> uint64_t *var_range_base, *fixed_range_base;
> int ret;
> + unsigned int index;
>
> var_range_base = (uint64_t *)v->arch.hvm.mtrr.var_ranges;
> fixed_range_base = (uint64_t *)v->arch.hvm.mtrr.fixed_ranges;
> @@ -3533,8 +3534,6 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
>
> switch ( msr )
> {
> - unsigned int index;
> -
> case MSR_EFER:
> *msr_content = v->arch.hvm.guest_efer;
> break;
> @@ -3636,6 +3635,7 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
> struct vcpu *v = current;
> struct domain *d = v->domain;
> int ret;
> + unsigned int index;
>
> HVMTRACE_3D(MSR_WRITE, msr,
> (uint32_t)msr_content, (uint32_t)(msr_content >> 32));
> @@ -3668,8 +3668,6 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
>
> switch ( msr )
> {
> - unsigned int index;
> -
> case MSR_EFER:
> if ( hvm_set_efer(msr_content) )
> return X86EMUL_EXCEPTION;
> diff --git a/xen/arch/x86/x86_emulate/0f01.c b/xen/arch/x86/x86_emulate/0f01.c
> index ba43fc394b..22a14b12c3 100644
> --- a/xen/arch/x86/x86_emulate/0f01.c
> +++ b/xen/arch/x86/x86_emulate/0f01.c
> @@ -24,13 +24,12 @@ int x86emul_0f01(struct x86_emulate_state *s,
> {
> enum x86_segment seg = (s->modrm_reg & 1) ? x86_seg_idtr : x86_seg_gdtr;
> int rc;
> + unsigned long base, limit, cr0, cr0w, cr4;
> + struct segment_register sreg;
> + uint64_t msr_val;
base and limit can go under case 0xfc
cr0 and cr0w can go under case GRP7_ALL(6)
> switch ( s->modrm )
> {
> - unsigned long base, limit, cr0, cr0w, cr4;
> - struct segment_register sreg;
> - uint64_t msr_val;
> -
> case 0xc6:
> switch ( s->vex.pfx )
> {
> diff --git a/xen/arch/x86/x86_emulate/blk.c b/xen/arch/x86/x86_emulate/blk.c
> index e790f4f900..f2956c0d52 100644
> --- a/xen/arch/x86/x86_emulate/blk.c
> +++ b/xen/arch/x86/x86_emulate/blk.c
> @@ -26,19 +26,18 @@ int x86_emul_blk(
> struct x86_emulate_ctxt *ctxt)
> {
> int rc = X86EMUL_OKAY;
> -
> - switch ( s->blk )
> - {
> - bool zf;
> + bool zf;
> #ifndef X86EMUL_NO_FPU
> + struct {
> + struct x87_env32 env;
> struct {
> - struct x87_env32 env;
> - struct {
> - uint8_t bytes[10];
> - } freg[8];
> - } fpstate;
> + uint8_t bytes[10];
> + } freg[8];
> + } fpstate;
> #endif
>
> + switch ( s->blk )
> + {
> /*
> * Throughout this switch(), memory clobbers are used to compensate
> * that other operands may not properly express the (full) memory
> diff --git a/xen/arch/x86/x86_emulate/decode.c b/xen/arch/x86/x86_emulate/decode.c
> index f58ca3984e..daebf3a9bd 100644
> --- a/xen/arch/x86/x86_emulate/decode.c
> +++ b/xen/arch/x86/x86_emulate/decode.c
> @@ -1758,9 +1758,9 @@ int x86emul_decode(struct x86_emulate_state *s,
> /* Fetch the immediate operand, if present. */
> switch ( d & SrcMask )
> {
> + case SrcImm: {
The Xen coding style is:
case SrcImm:
{
Also, this change looks wrong?
> unsigned int bytes;
>
> - case SrcImm:
> if ( !(d & ByteOp) )
> {
> if ( mode_64bit() && !amd_like(ctxt) &&
> @@ -1785,6 +1785,7 @@ int x86emul_decode(struct x86_emulate_state *s,
> case SrcImm16:
> s->imm1 = insn_fetch_type(uint16_t);
> break;
> + }
> }
>
> ctxt->opcode = opcode;
> diff --git a/xen/arch/x86/x86_emulate/fpu.c b/xen/arch/x86/x86_emulate/fpu.c
> index 480d879657..002d5e1253 100644
> --- a/xen/arch/x86/x86_emulate/fpu.c
> +++ b/xen/arch/x86/x86_emulate/fpu.c
> @@ -84,11 +84,10 @@ int x86emul_fpu(struct x86_emulate_state *s,
> uint8_t b;
> int rc;
> struct x86_emulate_stub stub = {};
> + unsigned long dummy;
>
> switch ( b = ctxt->opcode )
> {
> - unsigned long dummy;
> -
> case 0x9b: /* wait/fwait */
> host_and_vcpu_must_have(fpu);
> get_fpu(X86EMUL_FPU_wait);
> diff --git a/xen/arch/x86/x86_emulate/util-xen.c b/xen/arch/x86/x86_emulate/util-xen.c
> index 5e90818010..7ab2ac712f 100644
> --- a/xen/arch/x86/x86_emulate/util-xen.c
> +++ b/xen/arch/x86/x86_emulate/util-xen.c
> @@ -77,10 +77,10 @@ bool cf_check x86_insn_is_portio(const struct x86_emulate_state *s,
> bool cf_check x86_insn_is_cr_access(const struct x86_emulate_state *s,
> const struct x86_emulate_ctxt *ctxt)
> {
> + unsigned int ext;
> +
> switch ( ctxt->opcode )
> {
> - unsigned int ext;
This can go under case X86EMUL_OPC with a new block
> case X86EMUL_OPC(0x0f, 0x01):
> if ( x86_insn_modrm(s, NULL, &ext) >= 0
> && (ext & 5) == 4 ) /* SMSW / LMSW */
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
> index e88245eae9..d6c04fd5f3 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -1479,15 +1479,15 @@ x86_emulate(
> break;
> }
>
> + enum x86_segment seg;
> + struct segment_register cs, sreg;
> + struct cpuid_leaf leaf;
> + uint64_t msr_val;
> + unsigned int i, n;
> + unsigned long dummy;
> +
> switch ( ctxt->opcode )
> {
> - enum x86_segment seg;
> - struct segment_register cs, sreg;
> - struct cpuid_leaf leaf;
> - uint64_t msr_val;
> - unsigned int i, n;
> - unsigned long dummy;
In Xen we don't mix declarations and code, so they would have to go to
the top of the function
> case 0x00: case 0x01: add: /* add reg,mem */
> if ( ops->rmw && dst.type == OP_MEM )
> state->rmw = rmw_add;
> --
> 2.34.1
>
On 03.08.2023 04:33, Stefano Stabellini wrote:
> On Wed, 2 Aug 2023, Nicola Vetrini wrote:
>> @@ -2289,16 +2289,15 @@ static int cf_check hvmemul_cache_op(
>> struct hvm_emulate_ctxt *hvmemul_ctxt =
>> container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>> uint32_t pfec = PFEC_page_present;
>> + unsigned long addr;
>> + int rc;
>> + void *mapping;
>>
>> if ( !cache_flush_permitted(current->domain) )
>> return X86EMUL_OKAY;
>>
>> switch ( op )
>> {
>> - unsigned long addr;
>> - int rc;
>> - void *mapping;
>
> These three could be...
>
>
>> case x86emul_clflush:
>> case x86emul_clflushopt:
>> case x86emul_clwb:
>
> ... here in a new block
Except they're likely to be re-used as new enumerators are added.
>> --- a/xen/arch/x86/x86_emulate/util-xen.c
>> +++ b/xen/arch/x86/x86_emulate/util-xen.c
>> @@ -77,10 +77,10 @@ bool cf_check x86_insn_is_portio(const struct x86_emulate_state *s,
>> bool cf_check x86_insn_is_cr_access(const struct x86_emulate_state *s,
>> const struct x86_emulate_ctxt *ctxt)
>> {
>> + unsigned int ext;
>> +
>> switch ( ctxt->opcode )
>> {
>> - unsigned int ext;
>
> This can go under case X86EMUL_OPC with a new block
Same here.
Jan
© 2016 - 2026 Red Hat, Inc.