From b0ae508344062380e22f5f9306bf221ff38fc510 Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Fri, 30 Nov 2018 12:26:30 +0100
Subject: [PATCH] util: xml: Always consume args of virXMLFormatElement
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The function clears and frees the passed buffers on success, but not in
one case of failure. Modify the control flow that the args are always
consumed, record it in the docs and remove few pointless cleanup paths
in callers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/conf/domain_conf.c | 12 ++----------
 src/qemu/qemu_domain.c |  5 +----
 src/util/virxml.c      | 16 ++++++++++++----
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0c40a98f6b..b70dca6c61 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -24042,7 +24042,6 @@ virDomainDiskDefFormatIotune(virBufferPtr buf,
                              virDomainDiskDefPtr disk)
 {
     virBuffer childBuf = VIR_BUFFER_INITIALIZER;
-    int ret = -1;
 
     virBufferSetChildIndent(&childBuf, buf);
 
@@ -24077,10 +24076,7 @@ virDomainDiskDefFormatIotune(virBufferPtr buf,
     FORMAT_IOTUNE(read_iops_sec_max_length);
     FORMAT_IOTUNE(write_iops_sec_max_length);
 
-    ret = virXMLFormatElement(buf, "iotune", NULL, &childBuf);
-
-    virBufferFreeAndReset(&childBuf);
-    return ret;
+    return virXMLFormatElement(buf, "iotune", NULL, &childBuf);
 }
 
 #undef FORMAT_IOTUNE
@@ -24091,7 +24087,6 @@ virDomainDiskDefFormatDriver(virBufferPtr buf,
                              virDomainDiskDefPtr disk)
 {
     virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
-    int ret = -1;
 
     virBufferEscapeString(&driverBuf, " name='%s'", virDomainDiskGetDriver(disk));
 
@@ -24143,10 +24138,7 @@ virDomainDiskDefFormatDriver(virBufferPtr buf,
 
     virDomainVirtioOptionsFormat(&driverBuf, disk->virtio);
 
-    ret = virXMLFormatElement(buf, "driver", &driverBuf, NULL);
-
-    virBufferFreeAndReset(&driverBuf);
-    return ret;
+    return virXMLFormatElement(buf, "driver", &driverBuf, NULL);
 }
 
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0ef490be5a..9ed7115dab 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2234,14 +2234,11 @@ qemuDomainObjPrivateXMLFormatBlockjobs(virBufferPtr buf,
 {
     virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
     bool bj = qemuDomainHasBlockjob(vm, false);
-    int ret;
 
     virBufferAsprintf(&attrBuf, " active='%s'",
                       virTristateBoolTypeToString(virTristateBoolFromBool(bj)));
 
-    ret = virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL);
-    virBufferFreeAndReset(&attrBuf);
-    return ret;
+    return virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL);
 }
 
 
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 998d974882..3ed44e9036 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -1359,6 +1359,8 @@ virXMLValidatorFree(virXMLValidatorPtr validator)
  * @childBuf are NULL or are empty buffers the element is not
  * formatted.
  *
+ * Both passed buffers are always consumed and freed.
+ *
  * Returns 0 on success, -1 on error.
  */
 int
@@ -1367,15 +1369,16 @@ virXMLFormatElement(virBufferPtr buf,
                     virBufferPtr attrBuf,
                     virBufferPtr childBuf)
 {
+    int ret = -1;
+
     if ((!attrBuf || virBufferUse(attrBuf) == 0) &&
         (!childBuf || virBufferUse(childBuf) == 0)) {
         return 0;
     }
 
     if ((attrBuf && virBufferCheckError(attrBuf) < 0) ||
-        (childBuf && virBufferCheckError(childBuf) < 0)) {
-        return -1;
-    }
+        (childBuf && virBufferCheckError(childBuf) < 0))
+        goto cleanup;
 
     virBufferAsprintf(buf, "<%s", name);
 
@@ -1390,5 +1393,10 @@ virXMLFormatElement(virBufferPtr buf,
         virBufferAddLit(buf, "/>\n");
     }
 
-    return 0;
+    ret = 0;
+
+ cleanup:
+    virBufferFreeAndReset(attrBuf);
+    virBufferFreeAndReset(childBuf);
+    return ret;
 }