On Mon, Nov 10, 2025 at 09:39:50 +0100, Michal Privoznik via Devel wrote:
> From: Michal Privoznik <[email protected]>
>
> Domain capabilities XML is formatted (mostly) using
> FORMAT_PROLOGUE and FORMAT_EPILOGUE macros. These format opening
> and closing stanzas for given element. The FORMAT_PROLOGUE macro
> even tries to be clever and format element onto one line (if the
> element isn't supported), but that's not enough. Fortunately, we
> have virXMLFormatElement() which formats elements properly, so
> let's switch macros into using that.
>
> Signed-off-by: Michal Privoznik <[email protected]>
> ---
> src/conf/domain_capabilities.c | 25 ++++++++++-----------
> tests/domaincapsdata/bhyve_basic.x86_64.xml | 3 +--
> tests/domaincapsdata/bhyve_fbuf.x86_64.xml | 3 +--
> tests/domaincapsdata/bhyve_uefi.x86_64.xml | 3 +--
> 4 files changed, 15 insertions(+), 19 deletions(-)
>
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index 78a5b1f56a..be3c4002ab 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
> @@ -374,26 +374,25 @@ virDomainCapsStringValuesFormat(virBuffer *buf,
>
>
> #define FORMAT_PROLOGUE(item) \
> + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); \
> + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; \
Hmm, this isn't great as it pollutes the parent scope.
> do { \
And this bit is then definitely pointless as you definitely won't be
using this macro as a body of e.g. an 'if' without an explicit block.
> if (!item || item->supported == VIR_TRISTATE_BOOL_ABSENT) \
> return; \
> - virBufferAsprintf(buf, "<" #item " supported='%s'%s\n", \
> - (item->supported == VIR_TRISTATE_BOOL_YES) ? "yes" : "no", \
> - (item->supported == VIR_TRISTATE_BOOL_YES) ? ">" : "/>"); \
> - if (item->supported == VIR_TRISTATE_BOOL_NO) \
> + virBufferAsprintf(&attrBuf, " supported='%s'", \
> + (item->supported == VIR_TRISTATE_BOOL_YES) ? "yes"
> : "no"); \
> + if (item->supported == VIR_TRISTATE_BOOL_NO) { \
> + virXMLFormatElement(buf, #item, &attrBuf, NULL); \
> return; \
> - virBufferAdjustIndent(buf, 2); \
> + } \
> } while (0)
>
> #define FORMAT_EPILOGUE(item) \
> - do { \
> - virBufferAdjustIndent(buf, -2); \
> - virBufferAddLit(buf, "</" #item ">\n"); \
> - } while (0)
> + virXMLFormatElement(buf, #item, &attrBuf, &childBuf)
>
But I do like this change.
I wonder if adding some documentation about the expected usage would be
worth it.
Reviewed-by: Peter Krempa <[email protected]>
