| Message ID | 1581997168-20350-1-git-send-email-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: VMX: replace "fall through" with "return true" to indicate different case | expand |
On Tue, 2020-02-18 at 11:39 +0800, linmiaohe wrote: > The second "/* fall through */" in rmode_exception() makes code harder to > read. Replace it with "return true" to indicate they are different cases > and also this improves the readability. [] > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c [] > @@ -4495,7 +4495,7 @@ static bool rmode_exception(struct kvm_vcpu *vcpu, int vec) > if (vcpu->guest_debug & > (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) > return false; > - /* fall through */ > + return true; perhaps return !(vcpu->guest_debug & (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP));
linmiaohe <linmiaohe@huawei.com> writes: > From: Miaohe Lin <linmiaohe@huawei.com> > > The second "/* fall through */" in rmode_exception() makes code harder to > read. Replace it with "return true" to indicate they are different cases > and also this improves the readability. > > Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com> > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > arch/x86/kvm/vmx/vmx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index a13368b2719c..c5bcbbada2db 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -4495,7 +4495,7 @@ static bool rmode_exception(struct kvm_vcpu *vcpu, int vec) > if (vcpu->guest_debug & > (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) > return false; > - /* fall through */ > + return true; > case DE_VECTOR: > case OF_VECTOR: > case BR_VECTOR: Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> Thanks!
On Tue, Feb 18, 2020 at 11:39:28AM +0800, linmiaohe wrote: > From: Miaohe Lin <linmiaohe@huawei.com> > > The second "/* fall through */" in rmode_exception() makes code harder to > read. Replace it with "return true" to indicate they are different cases > and also this improves the readability. > > Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com> > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > arch/x86/kvm/vmx/vmx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index a13368b2719c..c5bcbbada2db 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -4495,7 +4495,7 @@ static bool rmode_exception(struct kvm_vcpu *vcpu, int vec) > if (vcpu->guest_debug & > (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) > return false; > - /* fall through */ > + return true; I prefer the current code, i.e. the fall through. This code is already burdened with a fall through, from #BP->#DB, and IMO the fall through makes it more obvious that the vcpu->guest_debug checks are corner cases, while everything else is handled by common logic. > case DE_VECTOR: > case OF_VECTOR: > case BR_VECTOR: > -- > 2.19.1 >
On 18/02/20 04:42, Joe Perches wrote: > On Tue, 2020-02-18 at 11:39 +0800, linmiaohe wrote: >> The second "/* fall through */" in rmode_exception() makes code harder to >> read. Replace it with "return true" to indicate they are different cases >> and also this improves the readability. > [] >> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > [] >> @@ -4495,7 +4495,7 @@ static bool rmode_exception(struct kvm_vcpu *vcpu, int vec) >> if (vcpu->guest_debug & >> (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) >> return false; >> - /* fall through */ >> + return true; > > perhaps > return !(vcpu->guest_debug & (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)); Nice, thanks Joe. Paolo
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index a13368b2719c..c5bcbbada2db 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -4495,7 +4495,7 @@ static bool rmode_exception(struct kvm_vcpu *vcpu, int vec) if (vcpu->guest_debug & (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) return false; - /* fall through */ + return true; case DE_VECTOR: case OF_VECTOR: case BR_VECTOR: