Re: [libvirt] [PATCH] libxl: unref objects in error paths

libxlMakeNic opens a virConnect object and takes a reference on a virNetwork object, but doesn't drop the references on all error paths. Rework the function to follow the standard libvirt pattern of using a local 'ret' variable to hold the function return value, performing all cleanup and returning 'ret' at a 'cleanup' label. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- I noticed these leaks while trying to rework the code to use a common virConnect object. That task has turned into quite an effort [1], but in the meantime the leaks in the current code should be plugged. [1] https://www.redhat.com/archives/libvir-list/2016-February/msg01188.html src/libxl/libxl_conf.c | 57 ++++++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index b20df29..763f4c5 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1286,7 +1286,11 @@ libxlMakeNic(virDomainDefPtr def, { bool ioemu_nic = def->os.type == VIR_DOMAIN_OSTYPE_HVM; virDomainNetType actual_type = virDomainNetGetActualType(l_nic); + char *brname = NULL; + virNetworkPtr network = NULL; + virConnectPtr conn = NULL; virNetDevBandwidthPtr actual_bw; + int ret = -1; /* TODO: Where is mtu stored? * @@ -1312,64 +1316,50 @@ libxlMakeNic(virDomainDefPtr def, if (l_nic->model) { if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) - return -1; + goto cleanup; if (STREQ(l_nic->model, "netfront")) x_nic->nictype = LIBXL_NIC_TYPE_VIF; } if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0) - return -1; + goto cleanup; switch (actual_type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: if (VIR_STRDUP(x_nic->bridge, virDomainNetGetActualBridgeName(l_nic)) < 0) - return -1; + goto cleanup; /* fallthrough */ case VIR_DOMAIN_NET_TYPE_ETHERNET: if (VIR_STRDUP(x_nic->script, l_nic->script) < 0) - return -1; + goto cleanup; if (l_nic->nips > 0) { x_nic->ip = virSocketAddrFormat(&l_nic->ips[0]->address); if (!x_nic->ip) - return -1; + goto cleanup; } break; case VIR_DOMAIN_NET_TYPE_NETWORK: { - bool fail = false; - char *brname = NULL; - virNetworkPtr network; - virConnectPtr conn; - if (!(conn = virConnectOpen("xen:///system"))) - return -1; + goto cleanup; if (!(network = virNetworkLookupByName(conn, l_nic->data.network.name))) { - virObjectUnref(conn); - return -1; + goto cleanup; } if (l_nic->nips > 0) { x_nic->ip = virSocketAddrFormat(&l_nic->ips[0]->address); if (!x_nic->ip) - return -1; + goto cleanup; } - if ((brname = virNetworkGetBridgeName(network))) { - if (VIR_STRDUP(x_nic->bridge, brname) < 0) - fail = true; - } else { - fail = true; - } - - VIR_FREE(brname); + if (!(brname = virNetworkGetBridgeName(network))) + goto cleanup; - virObjectUnref(network); - virObjectUnref(conn); - if (fail) - return -1; + if (VIR_STRDUP(x_nic->bridge, brname) < 0) + goto cleanup;

break; } case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -1385,18 +1375,18 @@ libxlMakeNic(virDomainDefPtr def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported interface type %s"), virDomainNetTypeToString(l_nic->type)); - return -1; + goto cleanup; } if (l_nic->domain_name) { #ifdef LIBXL_HAVE_DEVICE_BACKEND_DOMNAME if (VIR_STRDUP(x_nic->backend_domname, l_nic->domain_name) < 0) - return -1; + goto cleanup; #else virReportError(VIR_ERR_XML_DETAIL, "%s", _("this version of libxenlight does not " "support backend domain name")); - return -1; + goto cleanup; #endif } @@ -1438,7 +1428,14 @@ libxlMakeNic(virDomainDefPtr def, x_nic->rate_interval_usecs = 50000UL; } - return 0; + ret = 0; + + cleanup: + VIR_FREE(brname); + virObjectUnref(network); + virObjectUnref(conn); + + return ret; } static int