The return value of virXMLPropString was assigned into 'tmp' multiple times and to prevent static analyzers moaning about a potential leak a short-circuited if+logic or was used. Replace the code by having a helper variable for each possibility and also replace the for-loop to iterate elements. Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx> --- tools/virsh-domain.c | 48 +++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 8bd058a33a..4d8690d9db 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -12642,7 +12642,6 @@ virshFindDisk(const char *doc, g_autoptr(xmlXPathContext) ctxt = NULL; g_autofree xmlNodePtr *nodes = NULL; ssize_t nnodes; - xmlNodePtr cur = NULL; size_t i; xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt); @@ -12658,6 +12657,13 @@ virshFindDisk(const char *doc, /* search disk using @path */ for (i = 0; i < nnodes; i++) { + xmlNodePtr sourceNode; + g_autofree char *sourceFile = NULL; + g_autofree char *sourceDev = NULL; + g_autofree char *sourceDir = NULL; + g_autofree char *sourceName = NULL; + g_autofree char *targetDev = NULL; + if (type == VIRSH_FIND_DISK_CHANGEABLE) { g_autofree char *device = virXMLPropString(nodes[i], "device"); @@ -12668,29 +12674,25 @@ virshFindDisk(const char *doc, continue; } - cur = nodes[i]->children; - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE) { - g_autofree char *tmp = NULL; - - if (virXMLNodeNameEqual(cur, "source")) { - if ((tmp = virXMLPropString(cur, "file")) || - (tmp = virXMLPropString(cur, "dev")) || - (tmp = virXMLPropString(cur, "dir")) || - (tmp = virXMLPropString(cur, "name"))) { - } - } else if (virXMLNodeNameEqual(cur, "target")) { - tmp = virXMLPropString(cur, "dev"); - } + if ((sourceNode = virXMLNodeGetSubelement(nodes[i], "source"))) { + sourceFile = virXMLPropString(sourceNode, "file"); + sourceDev = virXMLPropString(sourceNode, "dev"); + sourceDir = virXMLPropString(sourceNode, "dir"); + sourceName = virXMLPropString(sourceNode, "name"); + } - if (STREQ_NULLABLE(tmp, path)) { - xmlNodePtr ret = xmlCopyNode(nodes[i], 1); - /* drop backing store since they are not needed here */ - virshDiskDropBackingStore(ret); - return ret; - } - } - cur = cur->next; + ctxt->node = nodes[i]; + targetDev = virXPathString("string(./target/@dev)", ctxt); + + if (STREQ_NULLABLE(targetDev, path) || + STREQ_NULLABLE(sourceFile, path) || + STREQ_NULLABLE(sourceDev, path) || + STREQ_NULLABLE(sourceDir, path) || + STREQ_NULLABLE(sourceName, path)) { + xmlNodePtr ret = xmlCopyNode(nodes[i], 1); + /* drop backing store since they are not needed here */ + virshDiskDropBackingStore(ret); + return ret; } } -- 2.38.1