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