Re: [libvirt] [PATCH] macvtap: plug memory leak for 802.1Qbh
On 10/14/2011 02:41 PM, Roopa Prabhu wrote:
On 10/13/11 3:49 PM, "Eric Blake"<eblake(a)redhat.com> wrote:
> Detected by Coverity. Leak present since commit ca3b22b.
>
> * src/util/macvtap.c (doPortProfileOp8021Qbh): Release device name.
> ---
> getPhysfnDev allocates physfndev, but nothing freed it.
> Pushing under the trivial rule.
Actually looks like physfndev is conditionally allocated in getPhysfnDev
Its better to modify getPhysfnDev to allocate physfndev every time.
Also a good catch - otherwise we might have a double free or otherwise
crash on freeing an invalid pointer.
I ACK this patch with the additional change below (looks ok ?)
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index a020c90..b50b7d2 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -964,7 +964,7 @@ getPhysfnDev(const char *linkdev,
*/
*vf = PORT_SELF_VF;
- *physfndev = (char *)linkdev;
+ *physfndev = strdup(linkdev);
I had already pushed mine. Yours is missing a virReportOOMError() on
allocation failure; but ACK with that improvement.
Meanwhile, we need to scrub this file - it uses a convention of 1 on
error, instead of the more typical -1 on error in the rest of the code
base; but I want this bug fix to be separate from that subsequent cleanup.
diff --git i/src/util/macvtap.c w/src/util/macvtap.c
index b50b7d2..33f0a13 100644
--- i/src/util/macvtap.c
+++ w/src/util/macvtap.c
@@ -965,6 +965,10 @@ getPhysfnDev(const char *linkdev,
*vf = PORT_SELF_VF;
*physfndev = strdup(linkdev);
+ if (!*physfndev) {
+ virReportOOMError();
+ rc = -1;
+ }
}
return rc;
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org