The variable points to a buffer not a domain object therefore its current name is misleading. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6a95344fd5..a7f659308c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9694,7 +9694,7 @@ static int qemuBuildSevCommandLine(virDomainObjPtr vm, virCommandPtr cmd, virDomainSevDefPtr sev) { - virBuffer obj = VIR_BUFFER_INITIALIZER; + virBuffer buf = VIR_BUFFER_INITIALIZER; qemuDomainObjPrivatePtr priv = vm->privateData; char *path = NULL; @@ -9704,25 +9704,25 @@ qemuBuildSevCommandLine(virDomainObjPtr vm, virCommandPtr cmd, VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d", sev->policy, sev->cbitpos, sev->reduced_phys_bits); - virBufferAsprintf(&obj, "sev-guest,id=sev0,cbitpos=%d", sev->cbitpos); - virBufferAsprintf(&obj, ",reduced-phys-bits=%d", sev->reduced_phys_bits); - virBufferAsprintf(&obj, ",policy=0x%x", sev->policy); + virBufferAsprintf(&buf, "sev-guest,id=sev0,cbitpos=%d", sev->cbitpos); + virBufferAsprintf(&buf, ",reduced-phys-bits=%d", sev->reduced_phys_bits); + virBufferAsprintf(&buf, ",policy=0x%x", sev->policy); if (sev->dh_cert) { if (virAsprintf(&path, "%s/dh_cert.base64", priv->libDir) < 0) return -1; - virBufferAsprintf(&obj, ",dh-cert-file=%s", path); + virBufferAsprintf(&buf, ",dh-cert-file=%s", path); VIR_FREE(path); } if (sev->session) { if (virAsprintf(&path, "%s/session.base64", priv->libDir) < 0) return -1; - virBufferAsprintf(&obj, ",session-file=%s", path); + virBufferAsprintf(&buf, ",session-file=%s", path); VIR_FREE(path); } - virCommandAddArgList(cmd, "-object", virBufferContentAndReset(&obj), NULL); + virCommandAddArgList(cmd, "-object", virBufferContentAndReset(&buf), NULL); return 0; } -- 2.16.4

The buffer is not freed anywhere. Nor in the error paths. Also the usage virCommand with respect to buffer is very odd. ==2504== 1,100 bytes in 1 blocks are definitely lost in loss record 167 of 175 ==2504== at 0x4C2CE3F: malloc (vg_replace_malloc.c:298) ==2504== by 0x4C2F1BF: realloc (vg_replace_malloc.c:785) ==2504== by 0x5D32EE2: virReallocN (viralloc.c:245) ==2504== by 0x5D37278: virBufferGrow (virbuffer.c:150) ==2504== by 0x5D3783E: virBufferVasprintf (virbuffer.c:408) ==2504== by 0x5D377A9: virBufferAsprintf (virbuffer.c:381) ==2504== by 0x57017C1: qemuBuildSevCommandLine (qemu_command.c:9707) ==2504== by 0x57030F7: qemuBuildCommandLine (qemu_command.c:10324) ==2504== by 0x575FA48: qemuProcessCreatePretendCmd (qemu_process.c:6644) ==2504== by 0x11351A: testCompareXMLToArgv (qemuxml2argvtest.c:564) ==2504== by 0x1392F7: virTestRun (testutils.c:180) ==2504== by 0x137895: mymain (qemuxml2argvtest.c:2900) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a7f659308c..796831f79c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9697,6 +9697,7 @@ qemuBuildSevCommandLine(virDomainObjPtr vm, virCommandPtr cmd, virBuffer buf = VIR_BUFFER_INITIALIZER; qemuDomainObjPrivatePtr priv = vm->privateData; char *path = NULL; + int ret = -1; if (!sev) return 0; @@ -9710,20 +9711,24 @@ qemuBuildSevCommandLine(virDomainObjPtr vm, virCommandPtr cmd, if (sev->dh_cert) { if (virAsprintf(&path, "%s/dh_cert.base64", priv->libDir) < 0) - return -1; + goto cleanup; virBufferAsprintf(&buf, ",dh-cert-file=%s", path); VIR_FREE(path); } if (sev->session) { if (virAsprintf(&path, "%s/session.base64", priv->libDir) < 0) - return -1; + goto cleanup; virBufferAsprintf(&buf, ",session-file=%s", path); VIR_FREE(path); } - virCommandAddArgList(cmd, "-object", virBufferContentAndReset(&buf), NULL); - return 0; + virCommandAddArg(cmd, "-object"); + virCommandAddArgBuffer(cmd, &buf); + ret = 0; + cleanup: + virBufferFreeAndReset(&buf); + return ret; } static int -- 2.16.4

Firstly, this function changes node for relative XPaths but doesn't restore the original one in case VIR_ALLOC(def) fails. Secondly, @type is leaked. Thirdly, dh-cert and session attributes are strdup()-ed needlessly, virXPathString already does that so we can use the retval immediately. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 85f07af46e..c788b525b5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15849,17 +15849,16 @@ static virDomainSevDefPtr virDomainSEVDefParseXML(xmlNodePtr sevNode, xmlXPathContextPtr ctxt) { - char *tmp = NULL; char *type = NULL; xmlNodePtr save = ctxt->node; virDomainSevDefPtr def; unsigned long policy; + if (VIR_ALLOC(def) < 0) + return NULL; + ctxt->node = sevNode; - if (VIR_ALLOC(def) < 0) - return NULL; - if (!(type = virXMLPropString(sevNode, "type"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing launch-security type")); @@ -15899,29 +15898,18 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode, } def->policy = policy; + def->dh_cert = virXPathString("string(./dh-cert)", ctxt); + def->session = virXPathString("string(./session)", ctxt); - if ((tmp = virXPathString("string(./dh-cert)", ctxt))) { - if (VIR_STRDUP(def->dh_cert, tmp) < 0) - goto error; - - VIR_FREE(tmp); - } - - if ((tmp = virXPathString("string(./session)", ctxt))) { - if (VIR_STRDUP(def->session, tmp) < 0) - goto error; - - VIR_FREE(tmp); - } - + cleanup: + VIR_FREE(type); ctxt->node = save; return def; error: - VIR_FREE(tmp); virDomainSEVDefFree(def); - ctxt->node = save; - return NULL; + def = NULL; + goto cleanup; } static virDomainMemoryDefPtr -- 2.16.4