[PATCH v2 0/8] some cleanups around CPU related code
Changes from v1, all suggested by Jano: - helper functions were added to virQEMUCapsLoadCache() to eliminate the cleanup label without mixing automatic and manual freeing - create extra strings with different scopes in virQEMUCapsLoadHostCPUModelInfo() to avoid mixing automatic and manual freeing - v1 link: https://listman.redhat.com/archives/libvir-list/2021-November/msg00497.html Daniel Henrique Barboza (8): qemu_capabilities.c: add virQEMUCapsParseFlags() qemu_capabilities.c: add virQEMUCapsParseGIC() qemu_capabilities.c: del 'nodes' var from virQEMUCapsLoadCache() qemu_capabilities.c: add virQEMUCapsValidateEmulator() qemu_capabilities.c: add virQEMUCapsValidateArch() qemu_capabilities.c: remove cleanup label from virQEMUCapsLoadCache() cpu_ppc64.c: remove 'guest' param from ppc64Compute() qemu_capabilities.c: del 'cleanup' label in virQEMUCapsLoadHostCPUModelInfo() src/cpu/cpu_ppc64.c | 26 +-- src/qemu/qemu_capabilities.c | 356 +++++++++++++++++++---------------- 2 files changed, 198 insertions(+), 184 deletions(-) -- 2.31.1
Create a new helper to remove the parse capabilities flag logic from the body of virQEMUCapsLoadCache(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_capabilities.c | 61 +++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 67fae46a34..61ec0727cb 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4098,6 +4098,44 @@ virQEMUCapsParseSEVInfo(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) } +static int +virQEMUCapsParseFlags(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) +{ + g_autofree xmlNodePtr *nodes = NULL; + size_t i; + int n; + + if ((n = virXPathNodeSet("./flag", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse qemu capabilities flags")); + return -1; + } + + VIR_DEBUG("Got flags %d", n); + for (i = 0; i < n; i++) { + g_autofree char *str = NULL; + int flag; + + if (!(str = virXMLPropString(nodes[i], "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing flag name in QEMU capabilities cache")); + return -1; + } + + flag = virQEMUCapsTypeFromString(str); + if (flag < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown qemu capabilities flag %s"), str); + return -1; + } + + virQEMUCapsSet(qemuCaps, flag); + } + + return 0; +} + + /* * Parsing a doc that looks like * @@ -4197,29 +4235,8 @@ virQEMUCapsLoadCache(virArch hostArch, if (virXPathLongLong("string(./qemumoddirmtime)", ctxt, &l) == 0) qemuCaps->modDirMtime = (time_t)l; - if ((n = virXPathNodeSet("./flag", ctxt, &nodes)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to parse qemu capabilities flags")); + if (virQEMUCapsParseFlags(qemuCaps, ctxt) < 0) goto cleanup; - } - VIR_DEBUG("Got flags %d", n); - for (i = 0; i < n; i++) { - int flag; - if (!(str = virXMLPropString(nodes[i], "name"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing flag name in QEMU capabilities cache")); - goto cleanup; - } - flag = virQEMUCapsTypeFromString(str); - if (flag < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown qemu capabilities flag %s"), str); - goto cleanup; - } - VIR_FREE(str); - virQEMUCapsSet(qemuCaps, flag); - } - VIR_FREE(nodes); if (virXPathUInt("string(./version)", ctxt, &qemuCaps->version) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", -- 2.31.1
Create a new helper to remove the GIC parse logic from the body of virQEMUCapsLoadCache(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_capabilities.c | 142 +++++++++++++++++++---------------- 1 file changed, 77 insertions(+), 65 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 61ec0727cb..38558a9ee0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4136,6 +4136,82 @@ virQEMUCapsParseFlags(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) } +static int +virQEMUCapsParseGIC(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) +{ + g_autofree xmlNodePtr *nodes = NULL; + size_t i; + int n; + + if ((n = virXPathNodeSet("./gic", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse qemu capabilities gic")); + return -1; + } + + if (n > 0) { + unsigned int uintValue; + bool boolValue; + + qemuCaps->ngicCapabilities = n; + qemuCaps->gicCapabilities = g_new0(virGICCapability, n); + + for (i = 0; i < n; i++) { + virGICCapability *cap = &qemuCaps->gicCapabilities[i]; + g_autofree char *version = NULL; + g_autofree char *kernel = NULL; + g_autofree char *emulated = NULL; + + if (!(version = virXMLPropString(nodes[i], "version"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing GIC version " + "in QEMU capabilities cache")); + return -1; + } + if (virStrToLong_ui(version, NULL, 10, &uintValue) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed GIC version " + "in QEMU capabilities cache")); + return -1; + } + cap->version = uintValue; + + if (!(kernel = virXMLPropString(nodes[i], "kernel"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing in-kernel GIC information " + "in QEMU capabilities cache")); + return -1; + } + if (!(boolValue = STREQ(kernel, "yes")) && STRNEQ(kernel, "no")) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed in-kernel GIC information " + "in QEMU capabilities cache")); + return -1; + } + if (boolValue) + cap->implementation |= VIR_GIC_IMPLEMENTATION_KERNEL; + + if (!(emulated = virXMLPropString(nodes[i], "emulated"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing emulated GIC information " + "in QEMU capabilities cache")); + return -1; + } + if (!(boolValue = STREQ(emulated, "yes")) && STRNEQ(emulated, "no")) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed emulated GIC information " + "in QEMU capabilities cache")); + return -1; + } + if (boolValue) + cap->implementation |= VIR_GIC_IMPLEMENTATION_EMULATED; + } + } + + return 0; +} + + /* * Parsing a doc that looks like * @@ -4164,8 +4240,6 @@ virQEMUCapsLoadCache(virArch hostArch, { g_autoptr(xmlDoc) doc = NULL; int ret = -1; - size_t i; - int n; xmlNodePtr *nodes = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; char *str = NULL; @@ -4293,70 +4367,8 @@ virQEMUCapsLoadCache(virArch hostArch, virQEMUCapsLoadAccel(qemuCaps, ctxt, VIR_DOMAIN_VIRT_QEMU) < 0) goto cleanup; - if ((n = virXPathNodeSet("./gic", ctxt, &nodes)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to parse qemu capabilities gic")); + if (virQEMUCapsParseGIC(qemuCaps, ctxt) < 0) goto cleanup; - } - if (n > 0) { - unsigned int uintValue; - bool boolValue; - - qemuCaps->ngicCapabilities = n; - qemuCaps->gicCapabilities = g_new0(virGICCapability, n); - - for (i = 0; i < n; i++) { - virGICCapability *cap = &qemuCaps->gicCapabilities[i]; - - if (!(str = virXMLPropString(nodes[i], "version"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing GIC version " - "in QEMU capabilities cache")); - goto cleanup; - } - if (virStrToLong_ui(str, NULL, 10, &uintValue) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("malformed GIC version " - "in QEMU capabilities cache")); - goto cleanup; - } - cap->version = uintValue; - VIR_FREE(str); - - if (!(str = virXMLPropString(nodes[i], "kernel"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing in-kernel GIC information " - "in QEMU capabilities cache")); - goto cleanup; - } - if (!(boolValue = STREQ(str, "yes")) && STRNEQ(str, "no")) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("malformed in-kernel GIC information " - "in QEMU capabilities cache")); - goto cleanup; - } - if (boolValue) - cap->implementation |= VIR_GIC_IMPLEMENTATION_KERNEL; - VIR_FREE(str); - - if (!(str = virXMLPropString(nodes[i], "emulated"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing emulated GIC information " - "in QEMU capabilities cache")); - goto cleanup; - } - if (!(boolValue = STREQ(str, "yes")) && STRNEQ(str, "no")) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("malformed emulated GIC information " - "in QEMU capabilities cache")); - goto cleanup; - } - if (boolValue) - cap->implementation |= VIR_GIC_IMPLEMENTATION_EMULATED; - VIR_FREE(str); - } - } - VIR_FREE(nodes); if (virQEMUCapsParseSEVInfo(qemuCaps, ctxt) < 0) goto cleanup; -- 2.31.1
The 'nodes' var is not being used. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_capabilities.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 38558a9ee0..2523a369d4 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4240,7 +4240,6 @@ virQEMUCapsLoadCache(virArch hostArch, { g_autoptr(xmlDoc) doc = NULL; int ret = -1; - xmlNodePtr *nodes = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; char *str = NULL; long long int l; @@ -4388,7 +4387,6 @@ virQEMUCapsLoadCache(virArch hostArch, ret = 0; cleanup: VIR_FREE(str); - VIR_FREE(nodes); return ret; } -- 2.31.1
Create a new helper to remove the emulator validation logic from the body of virQEMUCapsLoadCache(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_capabilities.c | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2523a369d4..a0a611da53 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4212,6 +4212,28 @@ virQEMUCapsParseGIC(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) } +static int +virQEMUCapsValidateEmulator(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) +{ + g_autofree char *str = NULL; + + if (!(str = virXPathString("string(./emulator)", ctxt))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing emulator in QEMU capabilities cache")); + return -1; + } + + if (STRNEQ(str, qemuCaps->binary)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Expected caps for '%s' but saw '%s'"), + qemuCaps->binary, str); + return -1; + } + + return 0; +} + + /* * Parsing a doc that looks like * @@ -4286,18 +4308,9 @@ virQEMUCapsLoadCache(virArch hostArch, goto cleanup; } - if (!(str = virXPathString("string(./emulator)", ctxt))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing emulator in QEMU capabilities cache")); - goto cleanup; - } - if (STRNEQ(str, qemuCaps->binary)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Expected caps for '%s' but saw '%s'"), - qemuCaps->binary, str); + if (virQEMUCapsValidateEmulator(qemuCaps, ctxt) < 0) goto cleanup; - } - VIR_FREE(str); + if (virXPathLongLong("string(./qemuctime)", ctxt, &l) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing qemuctime in QEMU capabilities XML")); -- 2.31.1
Create a new helper to remove the arch validation logic from the body of virQEMUCapsLoadCache(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_capabilities.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a0a611da53..6495cb028b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4234,6 +4234,26 @@ virQEMUCapsValidateEmulator(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) } +static int +virQEMUCapsValidateArch(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) +{ + g_autofree char *str = NULL; + + if (!(str = virXPathString("string(./arch)", ctxt))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing arch in QEMU capabilities cache")); + return -1; + } + if (!(qemuCaps->arch = virArchFromString(str))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown arch %s in QEMU capabilities cache"), str); + return -1; + } + + return 0; +} + + /* * Parsing a doc that looks like * @@ -4357,17 +4377,8 @@ virQEMUCapsLoadCache(virArch hostArch, goto cleanup; } - if (!(str = virXPathString("string(./arch)", ctxt))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing arch in QEMU capabilities cache")); + if (virQEMUCapsValidateArch(qemuCaps, ctxt) < 0) goto cleanup; - } - if (!(qemuCaps->arch = virArchFromString(str))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown arch %s in QEMU capabilities cache"), str); - goto cleanup; - } - VIR_FREE(str); if (virXPathBoolean("boolean(./cpudata)", ctxt) > 0) { qemuCaps->cpuData = virCPUDataParseNode(virXPathNode("./cpudata", ctxt)); -- 2.31.1
'str' is no longer being used and the 'cleanup' label can be removed together with the 'ret' variable. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_capabilities.c | 42 ++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6495cb028b..9567ab7662 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4281,17 +4281,15 @@ virQEMUCapsLoadCache(virArch hostArch, bool skipInvalidation) { g_autoptr(xmlDoc) doc = NULL; - int ret = -1; g_autoptr(xmlXPathContext) ctxt = NULL; - char *str = NULL; long long int l; unsigned long lu; if (!(doc = virXMLParseFile(filename))) - goto cleanup; + return -1; if (!(ctxt = virXMLXPathContextNew(doc))) - goto cleanup; + return -1; ctxt->node = xmlDocGetRootElement(doc); @@ -4300,13 +4298,13 @@ virQEMUCapsLoadCache(virArch hostArch, _("unexpected root element <%s>, " "expecting <qemuCaps>"), ctxt->node->name); - goto cleanup; + return -1; } if (virXPathLongLong("string(./selfctime)", ctxt, &l) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing selfctime in QEMU capabilities XML")); - goto cleanup; + return -1; } qemuCaps->libvirtCtime = (time_t)l; @@ -4324,17 +4322,16 @@ virQEMUCapsLoadCache(virArch hostArch, (long long)virGetSelfLastChanged(), (unsigned long)qemuCaps->libvirtVersion, (unsigned long)LIBVIR_VERSION_NUMBER); - ret = 1; - goto cleanup; + return 1; } if (virQEMUCapsValidateEmulator(qemuCaps, ctxt) < 0) - goto cleanup; + return -1; if (virXPathLongLong("string(./qemuctime)", ctxt, &l) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing qemuctime in QEMU capabilities XML")); - goto cleanup; + return -1; } qemuCaps->ctime = (time_t)l; @@ -4342,25 +4339,25 @@ virQEMUCapsLoadCache(virArch hostArch, qemuCaps->modDirMtime = (time_t)l; if (virQEMUCapsParseFlags(qemuCaps, ctxt) < 0) - goto cleanup; + return -1; if (virXPathUInt("string(./version)", ctxt, &qemuCaps->version) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing version in QEMU capabilities cache")); - goto cleanup; + return -1; } if (virXPathUInt("string(./kvmVersion)", ctxt, &qemuCaps->kvmVersion) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing version in QEMU capabilities cache")); - goto cleanup; + return -1; } if (virXPathUInt("string(./microcodeVersion)", ctxt, &qemuCaps->microcodeVersion) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing microcode version in QEMU capabilities cache")); - goto cleanup; + return -1; } qemuCaps->hostCPUSignature = virXPathString("string(./hostCPUSignature)", ctxt); @@ -4374,27 +4371,27 @@ virQEMUCapsLoadCache(virArch hostArch, if (virXPathBoolean("boolean(./kernelVersion)", ctxt) > 0) { qemuCaps->kernelVersion = virXPathString("string(./kernelVersion)", ctxt); if (!qemuCaps->kernelVersion) - goto cleanup; + return -1; } if (virQEMUCapsValidateArch(qemuCaps, ctxt) < 0) - goto cleanup; + return -1; if (virXPathBoolean("boolean(./cpudata)", ctxt) > 0) { qemuCaps->cpuData = virCPUDataParseNode(virXPathNode("./cpudata", ctxt)); if (!qemuCaps->cpuData) - goto cleanup; + return -1; } if (virQEMUCapsLoadAccel(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0 || virQEMUCapsLoadAccel(qemuCaps, ctxt, VIR_DOMAIN_VIRT_QEMU) < 0) - goto cleanup; + return -1; if (virQEMUCapsParseGIC(qemuCaps, ctxt) < 0) - goto cleanup; + return -1; if (virQEMUCapsParseSEVInfo(qemuCaps, ctxt) < 0) - goto cleanup; + return -1; virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM); virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU); @@ -4408,10 +4405,7 @@ virQEMUCapsLoadCache(virArch hostArch, if (skipInvalidation) qemuCaps->invalidation = false; - ret = 0; - cleanup: - VIR_FREE(str); - return ret; + return 0; } -- 2.31.1
ppc64Compute() is used only once, by virCPUppc64Compare(), which doesn't use the 'guest' parameter. It was last used by an API called 'cpuGuestData' that was dropped by commit 03fa904c0c0cb2. Removing the 'guest' parameter will not only remove unused code from ppc64Compute() but also remove the ppc64MakeCPUData() entirely. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index c7caaa9608..314d2f7c86 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -379,32 +379,15 @@ ppc64LoadMap(void) return g_steal_pointer(&map); } -static virCPUData * -ppc64MakeCPUData(virArch arch, - virCPUppc64Data *data) -{ - g_autoptr(virCPUData) cpuData = NULL; - - cpuData = g_new0(virCPUData, 1); - cpuData->arch = arch; - - if (ppc64DataCopy(&cpuData->data.ppc64, data) < 0) - return NULL; - - return g_steal_pointer(&cpuData); -} - static virCPUCompareResult ppc64Compute(virCPUDef *host, const virCPUDef *other, - virCPUData **guestData, char **message) { g_autoptr(virCPUppc64Map) map = NULL; g_autoptr(virCPUppc64Model) host_model = NULL; g_autoptr(virCPUppc64Model) guest_model = NULL; g_autoptr(virCPUDef) cpu = NULL; - virArch arch; size_t i; /* Ensure existing configurations are handled correctly */ @@ -431,9 +414,6 @@ ppc64Compute(virCPUDef *host, return VIR_CPU_COMPARE_INCOMPATIBLE; } - arch = cpu->arch; - } else { - arch = host->arch; } if (cpu->vendor && @@ -506,10 +486,6 @@ ppc64Compute(virCPUDef *host, return VIR_CPU_COMPARE_INCOMPATIBLE; } - if (guestData) - if (!(*guestData = ppc64MakeCPUData(arch, &guest_model->data))) - return VIR_CPU_COMPARE_ERROR; - return VIR_CPU_COMPARE_IDENTICAL; } @@ -532,7 +508,7 @@ virCPUppc64Compare(virCPUDef *host, return VIR_CPU_COMPARE_INCOMPATIBLE; } - ret = ppc64Compute(host, cpu, NULL, &message); + ret = ppc64Compute(host, cpu, &message); if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { ret = VIR_CPU_COMPARE_ERROR; -- 2.31.1
Create extra g_autofree strings and use them in an adequate scope to eliminate the need for VIR_FREE() calls and the 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_capabilities.c | 43 +++++++++++++++--------------------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9567ab7662..71dabdadd7 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3723,20 +3723,18 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, xmlXPathContextPtr ctxt, const char *typeStr) { - char *str = NULL; + g_autofree char *migratability = NULL; xmlNodePtr hostCPUNode; g_autofree xmlNodePtr *nodes = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autoptr(qemuMonitorCPUModelInfo) hostCPU = NULL; g_autofree char *xpath = g_strdup_printf("./hostCPU[@type='%s']", typeStr); - int ret = -1; size_t i; int n; int val; if (!(hostCPUNode = virXPathNode(xpath, ctxt))) { - ret = 0; - goto cleanup; + return 0; } hostCPU = g_new0(qemuMonitorCPUModelInfo, 1); @@ -3745,17 +3743,16 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing host CPU model name in QEMU " "capabilities cache")); - goto cleanup; + return -1; } - if (!(str = virXMLPropString(hostCPUNode, "migratability")) || - (val = virTristateBoolTypeFromString(str)) <= 0) { + if (!(migratability = virXMLPropString(hostCPUNode, "migratability")) || + (val = virTristateBoolTypeFromString(migratability)) <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid migratability value for host CPU model")); - goto cleanup; + return -1; } hostCPU->migratability = val == VIR_TRISTATE_BOOL_YES; - VIR_FREE(str); ctxt->node = hostCPUNode; @@ -3765,6 +3762,8 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, for (i = 0; i < n; i++) { qemuMonitorCPUProperty *prop = hostCPU->props + i; + g_autofree char *type = NULL; + g_autofree char *migratable = NULL; ctxt->node = nodes[i]; @@ -3772,17 +3771,16 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing 'name' attribute for a host CPU" " model property in QEMU capabilities cache")); - goto cleanup; + return -1; } - if (!(str = virXMLPropString(ctxt->node, "type")) || - (val = qemuMonitorCPUPropertyTypeFromString(str)) < 0) { + if (!(type = virXMLPropString(ctxt->node, "type")) || + (val = qemuMonitorCPUPropertyTypeFromString(type)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing or invalid CPU model property type " "in QEMU capabilities cache")); - goto cleanup; + return -1; } - VIR_FREE(str); prop->type = val; switch (prop->type) { @@ -3798,7 +3796,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, _("invalid string value for '%s' host CPU " "model property in QEMU capabilities cache"), prop->name); - goto cleanup; + return -1; } break; @@ -3809,7 +3807,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, _("invalid number value for '%s' host CPU " "model property in QEMU capabilities cache"), prop->name); - goto cleanup; + return -1; } break; @@ -3817,27 +3815,22 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, break; } - if ((str = virXMLPropString(ctxt->node, "migratable"))) { - if ((val = virTristateBoolTypeFromString(str)) <= 0) { + if ((migratable = virXMLPropString(ctxt->node, "migratable"))) { + if ((val = virTristateBoolTypeFromString(migratable)) <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown migratable value for '%s' host " "CPU model property"), prop->name); - goto cleanup; + return -1; } prop->migratable = val; - VIR_FREE(str); } } } caps->hostCPU.info = g_steal_pointer(&hostCPU); - ret = 0; - - cleanup: - VIR_FREE(str); - return ret; + return 0; } -- 2.31.1
On a Friday in 2021, Daniel Henrique Barboza wrote:
Changes from v1, all suggested by Jano:
- helper functions were added to virQEMUCapsLoadCache() to eliminate the cleanup label without mixing automatic and manual freeing - create extra strings with different scopes in virQEMUCapsLoadHostCPUModelInfo() to avoid mixing automatic and manual freeing - v1 link: https://listman.redhat.com/archives/libvir-list/2021-November/msg00497.html
Daniel Henrique Barboza (8): qemu_capabilities.c: add virQEMUCapsParseFlags() qemu_capabilities.c: add virQEMUCapsParseGIC() qemu_capabilities.c: del 'nodes' var from virQEMUCapsLoadCache() qemu_capabilities.c: add virQEMUCapsValidateEmulator() qemu_capabilities.c: add virQEMUCapsValidateArch() qemu_capabilities.c: remove cleanup label from virQEMUCapsLoadCache() cpu_ppc64.c: remove 'guest' param from ppc64Compute() qemu_capabilities.c: del 'cleanup' label in virQEMUCapsLoadHostCPUModelInfo()
src/cpu/cpu_ppc64.c | 26 +-- src/qemu/qemu_capabilities.c | 356 +++++++++++++++++++---------------- 2 files changed, 198 insertions(+), 184 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Daniel Henrique Barboza -
Ján Tomko