[PATCH 2/5] util: xml: Always consume args of virXMLFormatElement

[Peter Krempa <pkrempa@xxxxxxxxxx>]

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@xxxxxxxxxx>
---
 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 dd17174e25..77814a0c2a 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;
 }
-- 
2.19.2

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