There are few places in virDomainDefFormatFeatures() which can
use virXMLFormatElement() or virXMLFormatElementEmpty() instead
of writing directly into the output buffer.

After this, there are still a lot of places left, but that is
much bigger task.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/domain_conf.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cd87057524..060bd70de2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -27783,12 +27783,12 @@ virDomainDefFormatFeatures(virBuffer *buf,
                 break;
 
             case VIR_TRISTATE_SWITCH_ON:
-               virBufferAsprintf(&childBuf, "<%s state='on'/>\n", name);
-               break;
-
             case VIR_TRISTATE_SWITCH_OFF:
-               virBufferAsprintf(&childBuf, "<%s state='off'/>\n", name);
-               break;
+                virBufferAsprintf(&tmpAttrBuf, " state='%s'",
+                                  virTristateSwitchTypeToString(def->features[i]));
+
+                virXMLFormatElement(&childBuf, name, &tmpAttrBuf, NULL);
+                break;
             }
 
             break;
@@ -27816,12 +27816,12 @@ virDomainDefFormatFeatures(virBuffer *buf,
 
         case VIR_DOMAIN_FEATURE_APIC:
             if (def->features[i] == VIR_TRISTATE_SWITCH_ON) {
-                virBufferAddLit(&childBuf, "<apic");
                 if (def->apic_eoi) {
-                    virBufferAsprintf(&childBuf, " eoi='%s'",
+                    virBufferAsprintf(&tmpAttrBuf, " eoi='%s'",
                                       virTristateSwitchTypeToString(def->apic_eoi));
                 }
-                virBufferAddLit(&childBuf, "/>\n");
+
+                virXMLFormatElementEmpty(&childBuf, "apic", &tmpAttrBuf, NULL);
             }
             break;
 
@@ -27999,11 +27999,10 @@ virDomainDefFormatFeatures(virBuffer *buf,
 
         case VIR_DOMAIN_FEATURE_GIC:
             if (def->features[i] == VIR_TRISTATE_SWITCH_ON) {
-                virBufferAddLit(&childBuf, "<gic");
                 if (def->gic_version != VIR_GIC_VERSION_NONE)
-                    virBufferAsprintf(&childBuf, " version='%s'",
+                    virBufferAsprintf(&tmpAttrBuf, " version='%s'",
                                       virGICVersionTypeToString(def->gic_version));
-                virBufferAddLit(&childBuf, "/>\n");
+                virXMLFormatElementEmpty(&childBuf, "gic", &tmpAttrBuf, NULL);
             }
             break;
 
-- 
2.32.0

On Tue, Dec 14, 2021 at 03:01:19PM +0100, Michal Privoznik wrote:
>              case VIR_TRISTATE_SWITCH_ON:
> -               virBufferAsprintf(&childBuf, "<%s state='on'/>\n", name);
> -               break;
> -
>              case VIR_TRISTATE_SWITCH_OFF:
> -               virBufferAsprintf(&childBuf, "<%s state='off'/>\n", name);
> -               break;
> +                virBufferAsprintf(&tmpAttrBuf, " state='%s'",
> +                                  virTristateSwitchTypeToString(def->features[i]));
> +
> +                virXMLFormatElement(&childBuf, name, &tmpAttrBuf, NULL);
> +                break;

You even fixed indentation as a side effect! Very nice :)

>          case VIR_DOMAIN_FEATURE_GIC:
>              if (def->features[i] == VIR_TRISTATE_SWITCH_ON) {
> -                virBufferAddLit(&childBuf, "<gic");
>                  if (def->gic_version != VIR_GIC_VERSION_NONE)
> -                    virBufferAsprintf(&childBuf, " version='%s'",
> +                    virBufferAsprintf(&tmpAttrBuf, " version='%s'",
>                                        virGICVersionTypeToString(def->gic_version));
> -                virBufferAddLit(&childBuf, "/>\n");
> +                virXMLFormatElementEmpty(&childBuf, "gic", &tmpAttrBuf, NULL);

I think either adding an empty line before virXMLFormatElementEmpty()
or braces around the check on def->gic_version would improve
readability.

Regardless

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization