Re: [PATCH] kvm: device-assignment: Fix kvm_get_irq_route_gsi() return check

From: Alex Williamson <alex.williamson@hp.com>
To: "Yang, Sheng" <sheng.yang@intel.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: [PATCH] kvm: device-assignment: Fix kvm_get_irq_route_gsi() return check
Date: Tue, 12 May 2009 21:55:50 -0600	[thread overview]
Message-ID: <1242186950.4878.18.camel@2710p.home> (raw)
In-Reply-To: <200905131136.57555.sheng.yang@intel.com>

On Wed, 2009-05-13 at 11:36 +0800, Yang, Sheng wrote:
> On Wednesday 13 May 2009 06:14:01 Alex Williamson wrote:
> > --- a/hw/device-assignment.c
> > +++ b/hw/device-assignment.c
> > @@ -798,7 +798,7 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev,
> > unsigned int ctrl_pos) pci_dev->cap.start + PCI_MSI_DATA_32);
> >          assigned_dev->entry->type = KVM_IRQ_ROUTING_MSI;
> >          assigned_dev->entry->gsi = kvm_get_irq_route_gsi(kvm_context);
> > -        if (assigned_dev->entry->gsi < 0) {
> > +        if ((int)(assigned_dev->entry->gsi) < 0) {
> >              perror("assigned_dev_update_msi: kvm_get_irq_route_gsi");
> >              return;
> >          }
> 
> Use a return value(r) seems better.

Hi Sheng,

Do you mean:

r = kvm_get_irq_route_gsi(kvm_context);
if (r < 0) {
...
}
assigned_dev->entry->gsi = r;

> And I realized there is memory leak here. Entry seems haven't been freed for 
> error... So does MSI-X...

I hadn't noticed that one, but now that you mention it, yep.  Thanks,

Alex