[libvirt] [PATCH] tools: Tiny clean-ups for two functions in virsh-completer.c

Martin Kletzander posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/445e38374d2f031b9f5a677c40f6cd7ffb2f39e3.1526906861.git.mkletzan@redhat.com
Test syntax-check passed
tools/virsh-completer.c | 48 ++++++++++++++++++-----------------------
1 file changed, 21 insertions(+), 27 deletions(-)
[libvirt] [PATCH] tools: Tiny clean-ups for two functions in virsh-completer.c
Posted by Martin Kletzander 5 years, 10 months ago
These two functions were duplicating some cleanup paths, so let's just merge
both cleanup and error paths together.  To distinguish whether we need to
clean-up the return value let's keep it in @tmp until the function is successful
in which case we set @ret to the value of @tmp and set @tmp to NULL.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 tools/virsh-completer.c | 48 ++++++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index eb091070c92b..1f9f8e76e90e 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -97,6 +97,7 @@ virshDomainInterfaceCompleter(vshControl *ctl,
     size_t i;
     unsigned int domainXMLFlags = 0;
     char **ret = NULL;
+    char **tmp = NULL;
 
     virCheckFlags(VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC, NULL);
 
@@ -107,39 +108,35 @@ virshDomainInterfaceCompleter(vshControl *ctl,
         domainXMLFlags = VIR_DOMAIN_XML_INACTIVE;
 
     if (virshDomainGetXML(ctl, cmd, domainXMLFlags, &xmldoc, &ctxt) < 0)
-        goto error;
+        goto cleanup;
 
     ninterfaces = virXPathNodeSet("./devices/interface", ctxt, &interfaces);
     if (ninterfaces < 0)
-        goto error;
+        goto cleanup;
 
-    if (VIR_ALLOC_N(ret, ninterfaces + 1) < 0)
-        goto error;
+    if (VIR_ALLOC_N(tmp, ninterfaces + 1) < 0)
+        goto cleanup;
 
     for (i = 0; i < ninterfaces; i++) {
         ctxt->node = interfaces[i];
 
         if (!(flags & VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC) &&
-            (ret[i] = virXPathString("string(./target/@dev)", ctxt)))
+            (tmp[i] = virXPathString("string(./target/@dev)", ctxt)))
             continue;
 
         /* In case we are dealing with inactive domain XML there's no
          * <target dev=''/>. Offer MAC addresses then. */
-        if (!(ret[i] = virXPathString("string(./mac/@address)", ctxt)))
-            goto error;
+        if (!(tmp[i] = virXPathString("string(./mac/@address)", ctxt)))
+            goto cleanup;
     }
 
+    VIR_STEAL_PTR(ret, tmp);
+ cleanup:
     VIR_FREE(interfaces);
     xmlFreeDoc(xmldoc);
     xmlXPathFreeContext(ctxt);
+    virStringListFree(tmp);
     return ret;
-
- error:
-    VIR_FREE(interfaces);
-    xmlFreeDoc(xmldoc);
-    xmlXPathFreeContext(ctxt);
-    virStringListFree(ret);
-    return NULL;
 }
 
 
@@ -154,6 +151,7 @@ virshDomainDiskTargetCompleter(vshControl *ctl,
     xmlNodePtr *disks = NULL;
     int ndisks;
     size_t i;
+    char **tmp = NULL;
     char **ret = NULL;
 
     virCheckFlags(0, NULL);
@@ -162,32 +160,28 @@ virshDomainDiskTargetCompleter(vshControl *ctl,
         return NULL;
 
     if (virshDomainGetXML(ctl, cmd, 0, &xmldoc, &ctxt) < 0)
-        goto error;
+        goto cleanup;
 
     ndisks = virXPathNodeSet("./devices/disk", ctxt, &disks);
     if (ndisks < 0)
-        goto error;
+        goto cleanup;
 
-    if (VIR_ALLOC_N(ret, ndisks + 1) < 0)
-        goto error;
+    if (VIR_ALLOC_N(tmp, ndisks + 1) < 0)
+        goto cleanup;
 
     for (i = 0; i < ndisks; i++) {
         ctxt->node = disks[i];
-        if (!(ret[i] = virXPathString("string(./target/@dev)", ctxt)))
-            goto error;
+        if (!(tmp[i] = virXPathString("string(./target/@dev)", ctxt)))
+            goto cleanup;
     }
 
+    VIR_STEAL_PTR(ret, tmp);
+ cleanup:
     VIR_FREE(disks);
     xmlFreeDoc(xmldoc);
     xmlXPathFreeContext(ctxt);
+    virStringListFree(tmp);
     return ret;
-
- error:
-    VIR_FREE(disks);
-    xmlFreeDoc(xmldoc);
-    xmlXPathFreeContext(ctxt);
-    virStringListFree(ret);
-    return NULL;
 }
 
 
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tools: Tiny clean-ups for two functions in virsh-completer.c
Posted by Ján Tomko 5 years, 10 months ago
On Mon, May 21, 2018 at 02:47:41PM +0200, Martin Kletzander wrote:
>These two functions were duplicating some cleanup paths, so let's just merge
>both cleanup and error paths together.  To distinguish whether we need to
>clean-up the return value let's keep it in @tmp until the function is successful
>in which case we set @ret to the value of @tmp and set @tmp to NULL.
>
>Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>---
> tools/virsh-completer.c | 48 ++++++++++++++++++-----------------------
> 1 file changed, 21 insertions(+), 27 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

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