All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: liujing <liujing@cmss.chinamobile.com>
Cc: pbonzini@redhat.com, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, dave.hansen@linux.intel.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: Redundant variable assignments can be merged
Date: Wed, 30 Nov 2022 15:43:56 +0000	[thread overview]
Message-ID: <Y4d6PEGv4pvKyJeF@google.com> (raw)
In-Reply-To: <20221130115504.4282-1-liujing@cmss.chinamobile.com>

State what the patch does, not what can be done.  And "redundant" isn't the right
word to describe this.

On Wed, Nov 30, 2022, liujing wrote:
> When reading kvm code, find the 'r' variable declaration
> and then assign the value in kvm_vm_ioctl_get_irqchip and
> kvm_vm_ioctl_set_irqchip function, It can be combined into one sentence.

Why though?  I actually kinda like the existing code.

That said, what about removing the variable entirely?  That'd also avoid the
unnecessary call to kvm_pic_update_irq() in set's error path.

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d2ad383da998..898fce19430a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6011,9 +6011,7 @@ static unsigned long kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
 static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
 {
        struct kvm_pic *pic = kvm->arch.vpic;
-       int r;
 
-       r = 0;
        switch (chip->chip_id) {
        case KVM_IRQCHIP_PIC_MASTER:
                memcpy(&chip->chip.pic, &pic->pics[0],
@@ -6027,18 +6025,15 @@ static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
                kvm_get_ioapic(kvm, &chip->chip.ioapic);
                break;
        default:
-               r = -EINVAL;
-               break;
+               return -EINVAL;
        }
-       return r;
+       return 0;
 }
 
 static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
 {
        struct kvm_pic *pic = kvm->arch.vpic;
-       int r;
 
-       r = 0;
        switch (chip->chip_id) {
        case KVM_IRQCHIP_PIC_MASTER:
                spin_lock(&pic->lock);
@@ -6056,11 +6051,10 @@ static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
                kvm_set_ioapic(kvm, &chip->chip.ioapic);
                break;
        default:
-               r = -EINVAL;
-               break;
+               return -EINVAL;
        }
        kvm_pic_update_irq(pic);
-       return r;
+       return 0;
 }
 
 static int kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)


  reply	other threads:[~2022-11-30 15:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-30 11:55 [PATCH] KVM: x86: Redundant variable assignments can be merged liujing
2022-11-30 15:43 ` Sean Christopherson [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-11-30 12:59 liujing

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y4d6PEGv4pvKyJeF@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liujing@cmss.chinamobile.com \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.