On Tue, Mar 10, 2015 at 19:20:28 -0400, John Ferlan wrote:
Coverity complains that "net_set" is compared to NULL
before calling
xen_network_set_free, but used rather liberally before that. While
I was looking at the code I also noted that if the virAsprintfQuiet
fails, then we leak our structures - so I added those too.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/xenapi/xenapi_utils.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index ce09dfe..0c13745 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -399,14 +399,14 @@ createVifNetwork(virConnectPtr conn, xen_vm vm, int device,
xen_network_set *net_set = NULL;
xen_network_record *net_rec = NULL;
int cnt = 0;
- if (xen_network_get_all(session, &net_set)) {
- for (cnt = 0; cnt < net_set->size; cnt++) {
- if (xen_network_get_record(session, &net_rec,
net_set->contents[cnt])) {
- if (STREQ(net_rec->bridge, bridge)) {
- break;
- } else {
- xen_network_record_free(net_rec);
- }
+ if (!xen_network_get_all(session, &net_set))
+ return -1
missing semicolon?
Also @vm_opt is allocated in the declarations section and would be
leaked here. Pre-existing though, so I won't insist on fixing that too.
+ for (cnt = 0; cnt < net_set->size; cnt++) {
+ if (xen_network_get_record(session, &net_rec, net_set->contents[cnt])) {
+ if (STREQ(net_rec->bridge, bridge)) {
+ break;
+ } else {
+ xen_network_record_free(net_rec);
}
}
}
@@ -425,8 +425,13 @@ createVifNetwork(virConnectPtr conn, xen_vm vm, int device,
vif_record->other_config = xen_string_string_map_alloc(0);
vif_record->runtime_properties = xen_string_string_map_alloc(0);
vif_record->qos_algorithm_params = xen_string_string_map_alloc(0);
- if (virAsprintfQuiet(&vif_record->device, "%d", device) <
0)
+ if (virAsprintfQuiet(&vif_record->device, "%d", device) < 0)
{
+ xen_vif_free(vif);
@vif is allocated few lines below so no need to free it yet.
+ xen_vif_record_free(vif_record);
+ xen_network_record_free(net_rec);
+ xen_network_set_free(net_set);
return -1;
+ }
xen_vif_create(session, &vif, vif_record);
if (!vif) {
xen_vif_free(vif);
@@ -438,7 +443,7 @@ createVifNetwork(virConnectPtr conn, xen_vm vm, int device,
xen_vif_record_free(vif_record);
xen_network_record_free(net_rec);
}
- if (net_set != NULL) xen_network_set_free(net_set);
+ xen_network_set_free(net_set);
return -1;
}
Looks good, ACK if you compile test this patch before pushing, with or
without fixing of the other existing leak.
Peter