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@xxxxxxxxxx>
--
Andrea Bolognani / Red Hat / Virtualization
