We've been open-coding virStringListFreeCount for cleaning up
the completion list we're building. This had the advantage of
zeoring the pointer afterwards, which is no longer needed
now that we compile the list in 'tmp' instead of 'ret'.

Since all our lists are NULL-terminated anyway, switch to using
virStringListFree which has the benefit of being usable with
our VIR_AUTOSTRINGLIST macro.

Fixes nearly impossible NULL dereferences in
  virshNWFilterBindingNameCompleter
  virshNWFilterNameCompleter
  virshNodeDeviceNameCompleter
  virshNetworkNameCompleter
  virshInterfaceNameCompleter
  virshStoragePoolNameCompleter
  virshDomainNameCompleter
which jumped on the error label after a failed allocation
and a possible one in
  virshStorageVolNameCompleter
which jumped there when we fail to fetch the list of volumes.

Signed-off-by: Ján Tomko <jto...@redhat.com>
---
 tools/virsh-completer.c | 67 +++++++++++------------------------------
 1 file changed, 18 insertions(+), 49 deletions(-)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 9d56659259..20b325c020 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -112,12 +112,10 @@ virshDomainNameCompleter(vshControl *ctl,
     for (i = 0; i < ndomains; i++)
         virshDomainFree(domains[i]);
     VIR_FREE(domains);
+    virStringListFree(tmp);
     return ret;
 
  error:
-    for (i = 0; i < ndomains; i++)
-        VIR_FREE(tmp[i]);
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
@@ -262,12 +260,10 @@ virshStoragePoolNameCompleter(vshControl *ctl,
     for (i = 0; i < npools; i++)
         virStoragePoolFree(pools[i]);
     VIR_FREE(pools);
+    virStringListFree(tmp);
     return ret;
 
  error:
-    for (i = 0; i < npools; i++)
-        VIR_FREE(tmp[i]);
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
@@ -315,12 +311,10 @@ virshStorageVolNameCompleter(vshControl *ctl,
     for (i = 0; i < nvols; i++)
         virStorageVolFree(vols[i]);
     VIR_FREE(vols);
+    virStringListFree(tmp);
     return ret;
 
  error:
-    for (i = 0; i < nvols; i++)
-        VIR_FREE(tmp[i]);
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
@@ -363,12 +357,10 @@ virshInterfaceNameCompleter(vshControl *ctl,
     for (i = 0; i < nifaces; i++)
         virInterfaceFree(ifaces[i]);
     VIR_FREE(ifaces);
+    virStringListFree(tmp);
     return ret;
 
  error:
-    for (i = 0; i < nifaces; i++)
-        VIR_FREE(tmp[i]);
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
@@ -412,12 +404,10 @@ virshNetworkNameCompleter(vshControl *ctl,
     for (i = 0; i < nnets; i++)
         virNetworkFree(nets[i]);
     VIR_FREE(nets);
+    virStringListFree(tmp);
     return ret;
 
  error:
-    for (i = 0; i < nnets; i++)
-        VIR_FREE(tmp[i]);
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
@@ -444,10 +434,10 @@ virshNetworkEventNameCompleter(vshControl *ctl 
ATTRIBUTE_UNUSED,
     VIR_STEAL_PTR(ret, tmp);
 
  cleanup:
+    virStringListFree(tmp);
     return ret;
 
  error:
-    virStringListFree(tmp);
     goto cleanup;
 }
 
@@ -488,12 +478,10 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
     for (i = 0; i < ndevs; i++)
         virNodeDeviceFree(devs[i]);
     VIR_FREE(devs);
+    virStringListFree(tmp);
     return ret;
 
  error:
-    for (i = 0; i < ndevs; i++)
-        VIR_FREE(tmp[i]);
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
@@ -534,12 +522,10 @@ virshNWFilterNameCompleter(vshControl *ctl,
     for (i = 0; i < nnwfilters; i++)
         virNWFilterFree(nwfilters[i]);
     VIR_FREE(nwfilters);
+    virStringListFree(tmp);
     return ret;
 
  error:
-    for (i = 0; i < nnwfilters; i++)
-        VIR_FREE(tmp[i]);
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
@@ -580,12 +566,10 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
     for (i = 0; i < nbindings; i++)
         virNWFilterBindingFree(bindings[i]);
     VIR_FREE(bindings);
+    virStringListFree(tmp);
     return ret;
 
  error:
-    for (i = 0; i < nbindings; i++)
-        VIR_FREE(tmp[i]);
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
@@ -627,12 +611,10 @@ virshSecretUUIDCompleter(vshControl *ctl,
     for (i = 0; i < nsecrets; i++)
         virSecretFree(secrets[i]);
     VIR_FREE(secrets);
+    virStringListFree(tmp);
     return ret;
 
  error:
-    for (i = 0; i < nsecrets; i++)
-        VIR_FREE(tmp[i]);
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
@@ -680,12 +662,10 @@ virshSnapshotNameCompleter(vshControl *ctl,
     for (i = 0; i < nsnapshots; i++)
         virshDomainSnapshotFree(snapshots[i]);
     VIR_FREE(snapshots);
+    virStringListFree(tmp);
     return ret;
 
  error:
-    for (i = 0; i < nsnapshots; i++)
-        VIR_FREE(tmp[i]);
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
@@ -764,15 +744,10 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
     VIR_FREE(pagesize);
     VIR_FREE(cap_xml);
     VIR_FREE(unit);
-
+    virStringListFree(tmp);
     return ret;
 
  error:
-    if (tmp) {
-        for (i = 0; i < npages; i++)
-            VIR_FREE(tmp[i]);
-    }
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
@@ -799,10 +774,10 @@ virshSecretEventNameCompleter(vshControl *ctl 
ATTRIBUTE_UNUSED,
     VIR_STEAL_PTR(ret, tmp);
 
  cleanup:
+    virStringListFree(tmp);
     return ret;
 
  error:
-    virStringListFree(tmp);
     goto cleanup;
 }
 
@@ -829,10 +804,10 @@ virshDomainEventNameCompleter(vshControl *ctl 
ATTRIBUTE_UNUSED,
     VIR_STEAL_PTR(ret, tmp);
 
  cleanup:
+    virStringListFree(tmp);
     return ret;
 
  error:
-    virStringListFree(tmp);
     goto cleanup;
 }
 
@@ -859,10 +834,10 @@ virshPoolEventNameCompleter(vshControl *ctl 
ATTRIBUTE_UNUSED,
     VIR_STEAL_PTR(ret, tmp);
 
  cleanup:
+    virStringListFree(tmp);
     return ret;
 
  error:
-    virStringListFree(tmp);
     goto cleanup;
 }
 
@@ -933,11 +908,10 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
     VIR_FREE(interfaces);
     xmlXPathFreeContext(ctxt);
     xmlFreeDoc(xml);
+    virStringListFree(tmp);
     return ret;
 
  error:
-    virStringListFree(tmp);
-    tmp = NULL;
     goto cleanup;
 }
 
@@ -964,10 +938,10 @@ virshNodedevEventNameCompleter(vshControl *ctl 
ATTRIBUTE_UNUSED,
     VIR_STEAL_PTR(ret, tmp);
 
  cleanup:
+    virStringListFree(tmp);
     return ret;
 
  error:
-    virStringListFree(tmp);
     goto cleanup;
 }
 
@@ -1017,15 +991,10 @@ virshCellnoCompleter(vshControl *ctl,
     VIR_FREE(cells);
     xmlFreeDoc(doc);
     VIR_FREE(cap_xml);
-
+    virStringListFree(tmp);
     return ret;
 
  error:
-    if (tmp) {
-        for (i = 0; i < ncells; i++)
-            VIR_FREE(tmp[i]);
-    }
-    VIR_FREE(tmp);
     goto cleanup;
 }
 
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to