[PATCH 3/4] conf: Rework <tpm/> formatting

[Michal Privoznik]

The <tpm/> element formatting is handled in
virDomainTPMDefFormat() which uses the "old style" - appending
strings directly into the output buffer. With this, it's easy to
get conditions that tell when an element has ended wrong. In this
particular case, if both <encryption/> and <active_pcr_banks/>
are to be formatted the current code puts a stray '>' into the
output buffer, resulting in invalid XML.

Rewrite the function to use virXMLFormatElement() which is more
clever.

https://bugzilla.redhat.com/show_bug.cgi?id=2016599#c15

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/conf/domain_conf.c                       | 53 ++++++++------------
 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml |  1 -
 2 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 716c6d2240..b8fef8586c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -25481,63 +25481,54 @@ virDomainTPMDefFormat(virBuffer *buf,
                       virDomainTPMDef *def,
                       unsigned int flags)
 {
-    virBufferAsprintf(buf, "<tpm model='%s'>\n",
+    g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
+    g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+    g_auto(virBuffer) backendAttrBuf = VIR_BUFFER_INITIALIZER;
+    g_auto(virBuffer) backendBuf = VIR_BUFFER_INIT_CHILD(&childBuf);
+
+    virBufferAsprintf(&attrBuf, " model='%s'",
                       virDomainTPMModelTypeToString(def->model));
-    virBufferAdjustIndent(buf, 2);
-    virBufferAsprintf(buf, "<backend type='%s'",
+
+    virBufferAsprintf(&backendAttrBuf, " type='%s'",
                       virDomainTPMBackendTypeToString(def->type));
 
     switch (def->type) {
     case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-        virBufferAddLit(buf, ">\n");
-        virBufferAdjustIndent(buf, 2);
-        virBufferEscapeString(buf, "<device path='%s'/>\n",
+        virBufferEscapeString(&backendBuf, "<device path='%s'/>\n",
                               def->data.passthrough.source->data.file.path);
-        virBufferAdjustIndent(buf, -2);
-        virBufferAddLit(buf, "</backend>\n");
         break;
     case VIR_DOMAIN_TPM_TYPE_EMULATOR:
-        virBufferAsprintf(buf, " version='%s'",
+        virBufferAsprintf(&backendAttrBuf, " version='%s'",
                           virDomainTPMVersionTypeToString(def->version));
         if (def->data.emulator.persistent_state)
-            virBufferAddLit(buf, " persistent_state='yes'");
+            virBufferAddLit(&backendAttrBuf, " persistent_state='yes'");
         if (def->data.emulator.hassecretuuid) {
             char uuidstr[VIR_UUID_STRING_BUFLEN];
-            virBufferAddLit(buf, ">\n");
-            virBufferAdjustIndent(buf, 2);
-            virBufferAsprintf(buf, "<encryption secret='%s'/>\n",
-                virUUIDFormat(def->data.emulator.secretuuid, uuidstr));
-            virBufferAdjustIndent(buf, -2);
+
+            virBufferAsprintf(&backendBuf, "<encryption secret='%s'/>\n",
+                              virUUIDFormat(def->data.emulator.secretuuid, uuidstr));
         }
         if (def->data.emulator.activePcrBanks) {
+            g_auto(virBuffer) activePcrBanksBuf = VIR_BUFFER_INIT_CHILD(&backendBuf);
             size_t i;
-            virBufferAddLit(buf, ">\n");
-            virBufferAdjustIndent(buf, 2);
-            virBufferAddLit(buf, "<active_pcr_banks>\n");
-            virBufferAdjustIndent(buf, 2);
+
             for (i = VIR_DOMAIN_TPM_PCR_BANK_SHA1; i < VIR_DOMAIN_TPM_PCR_BANK_LAST; i++) {
                 if ((def->data.emulator.activePcrBanks & (1 << i)))
-                    virBufferAsprintf(buf, "<%s/>\n",
+                    virBufferAsprintf(&activePcrBanksBuf, "<%s/>\n",
                                       virDomainTPMPcrBankTypeToString(i));
             }
-            virBufferAdjustIndent(buf, -2);
-            virBufferAddLit(buf, "</active_pcr_banks>\n");
-            virBufferAdjustIndent(buf, -2);
+
+            virXMLFormatElement(&backendBuf, "active_pcr_banks", NULL, &activePcrBanksBuf);
         }
-        if (def->data.emulator.hassecretuuid ||
-            def->data.emulator.activePcrBanks)
-            virBufferAddLit(buf, "</backend>\n");
-        else
-            virBufferAddLit(buf, "/>\n");
         break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
         break;
     }
 
-    virDomainDeviceInfoFormat(buf, &def->info, flags);
+    virXMLFormatElement(&childBuf, "backend", &backendAttrBuf, &backendBuf);
+    virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
 
-    virBufferAdjustIndent(buf, -2);
-    virBufferAddLit(buf, "</tpm>\n");
+    virXMLFormatElement(buf, "tpm", &attrBuf, &childBuf);
 
     return 0;
 }
diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
index 59dd68311f..79acde218b 100644
--- a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
+++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
@@ -30,7 +30,6 @@
     <tpm model='tpm-tis'>
       <backend type='emulator' version='2.0'>
         <encryption secret='b4a117f1-8af2-44a4-91b8-7f0d2d4d68a3'/>
-      >
         <active_pcr_banks>
           <sha256/>
           <sha512/>
-- 
2.34.1