[libvirt] [PATCH 07/14] virsh-completer: virStringListFree where possible

Ján Tomko jtomko at redhat.com
Mon Apr 1 07:33:24 UTC 2019


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 <jtomko at 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




More information about the libvir-list mailing list