util: xml: Always consume args of virXMLFormatElement

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>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0c40a98f6bd710f407b8f178b6b9fb8e609cf53d..b70dca6c619cc3613f2ec8cbc11a580a25dc8c09 100644 (file)
--- 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 0ef490be5a97bcc325eb2ce705cd369f6afed03f..9ed7115dab197128ca38f595a4af407452a7e4f5 100644 (file)
--- 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 998d9748823cfeb240249efd5d2faba5d3a14fc4..3ed44e9036cfd21dde2c8f10a996813c167ca2ca 100644 (file)
--- 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;
 }