Re: [libvirt] [PATCH 7/8] conf: check for buffer errors before virBufferUse

> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c > index 0f99f3096..db7efffdf 100644 > --- a/src/conf/capabilities.c > +++ b/src/conf/capabilities.c > @@ -930,6 +930,11 @@ virCapabilitiesFormatCaches(virBufferPtr buf, > bank->controls[j]->max_allocation); > } > One could move the VIR_FREE(cpus_str) to right here and avoid the two

It also seems this particular function uses virBufferAdjustIndent on buf instead of on the child buf (or in this case controlBuf)...

> + if (virBufferCheckError(&controlBuf) < 0) { > + VIR_FREE(cpus_str); > + return -1; > + } > + > if (virBufferUse(&controlBuf)) { > virBufferAddLit(buf, ">\n"); > virBufferAddBuffer(buf, &controlBuf); [...] > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 7728321cb..4dc49fdb0 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c [...] > @@ -23309,6 +23321,10 @@ static int virDomainPanicDefFormat(virBufferPtr buf, > virBufferAdjustIndent(&childrenBuf, indent + 2); > if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0) > return -1; > + > + if (virBufferCheckError(&childrenBuf) < 0) > + return -1; > + Seeing a virBufferFreeAndReset below this if condition first had me thinking, well that's unnecessary; however, in actuality I think we have a leak any time virBufferUse doesn't return a non zero value call virBufferAddBuffer to consume the buffer.

Not necessarily something to stop this patch, but perhaps a leak for: virDomainDiskDefFormat virDomainControllerDriverFormat virDomainControllerDefFormat virDomainFSDefFormat virDomainMemballoonDefFormat virDomainRNGDefFormat virDomainVideoDefFormat virDomainInputDefFormat virDomainIOMMUDefFormat virDomainDiskDefFormat > if (virBufferUse(&childrenBuf)) { > virBufferAddLit(buf, ">\n"); > virBufferAddBuffer(buf, &childrenBuf); > @@ -23655,6 +23671,9 @@ virDomainInputDefFormat(virBufferPtr buf, [...]