[libvirt PATCH 0/6] qemu: capabilities: use g_auto (glib chronicles)
This is not a cover letter. Ján Tomko (6): qemu: refactor virQEMUCapsNewForBinaryInternal qemu: refactor virQEMUCapsLoadFile qemu: refactor virQEMUCapsInit qemu: refactor virQEMUCapsNewCopy qemu: capabilities: use g_auto qemu: capabilities: remove pointless labels src/qemu/qemu_capabilities.c | 134 ++++++++++++----------------------- 1 file changed, 45 insertions(+), 89 deletions(-) -- 2.31.1
Use g_auto and remove pointless labels. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f6f4ee28fb..174352a5a2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5430,18 +5430,18 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, unsigned int microcodeVersion, const char *kernelVersion) { - virQEMUCaps *qemuCaps; + g_autoptr(virQEMUCaps) qemuCaps = NULL; struct stat sb; if (!(qemuCaps = virQEMUCapsNewBinary(binary))) - goto error; + return NULL; /* We would also want to check faccessat if we cared about ACLs, * but we don't. */ if (stat(binary, &sb) < 0) { virReportSystemError(errno, _("Cannot check QEMU binary %s"), binary); - goto error; + return NULL; } qemuCaps->ctime = sb.st_ctime; @@ -5452,20 +5452,20 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, if (!virFileIsExecutable(binary)) { virReportSystemError(errno, _("QEMU binary %s is not executable"), binary); - goto error; + return NULL; } if (virFileExists(QEMU_MODDIR)) { if (stat(QEMU_MODDIR, &sb) < 0) { virReportSystemError(errno, _("Cannot check QEMU module directory %s"), QEMU_MODDIR); - goto error; + return NULL; } qemuCaps->modDirMtime = sb.st_mtime; } if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid) < 0) - goto error; + return NULL; qemuCaps->libvirtCtime = virGetSelfLastChanged(); qemuCaps->libvirtVersion = LIBVIR_VERSION_NUMBER; @@ -5484,11 +5484,7 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, qemuCaps->kvmSupportsSecureGuest = virQEMUCapsKVMSupportsSecureGuest(); } - return qemuCaps; - - error: - virObjectUnref(qemuCaps); - return NULL; + return g_steal_pointer(&qemuCaps); } static void * -- 2.31.1
Use g_auto and remove pointless labels. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 174352a5a2..a80c172d71 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5510,7 +5510,7 @@ virQEMUCapsLoadFile(const char *filename, void *privData, bool *outdated) { - virQEMUCaps *qemuCaps = virQEMUCapsNewBinary(binary); + g_autoptr(virQEMUCaps) qemuCaps = virQEMUCapsNewBinary(binary); virQEMUCapsCachePriv *priv = privData; int ret; @@ -5519,17 +5519,13 @@ virQEMUCapsLoadFile(const char *filename, ret = virQEMUCapsLoadCache(priv->hostArch, qemuCaps, filename, false); if (ret < 0) - goto error; + return NULL; if (ret == 1) { *outdated = true; - goto error; + return NULL; } - return qemuCaps; - - error: - virObjectUnref(qemuCaps); - return NULL; + return g_steal_pointer(&qemuCaps); } -- 2.31.1
Use g_auto and remove pointless labels. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a80c172d71..380d0a5694 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1111,13 +1111,13 @@ virQEMUCapsProbeHostCPU(virArch hostArch, virCaps * virQEMUCapsInit(virFileCache *cache) { - virCaps *caps; + g_autoptr(virCaps) caps = NULL; size_t i; virArch hostarch = virArchFromHost(); if ((caps = virCapabilitiesNew(hostarch, true, true)) == NULL) - goto error; + return NULL; if (virCapabilitiesInitCaches(caps) < 0) VIR_WARN("Failed to get host CPU cache info"); @@ -1145,13 +1145,9 @@ virQEMUCapsInit(virFileCache *cache) if (virQEMUCapsInitGuest(caps, cache, hostarch, i) < 0) - goto error; + return NULL; - return caps; - - error: - virObjectUnref(caps); - return NULL; + return g_steal_pointer(&caps); } -- 2.31.1
Use g_auto and remove pointless labels. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 380d0a5694..fecd332604 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1953,7 +1953,7 @@ virQEMUCapsAccelCopy(virQEMUCapsAccel *dst, virQEMUCaps *virQEMUCapsNewCopy(virQEMUCaps *qemuCaps) { - virQEMUCaps *ret = virQEMUCapsNewBinary(qemuCaps->binary); + g_autoptr(virQEMUCaps) ret = virQEMUCapsNewBinary(qemuCaps->binary); size_t i; if (!ret) @@ -1980,7 +1980,7 @@ virQEMUCaps *virQEMUCapsNewCopy(virQEMUCaps *qemuCaps) if (virQEMUCapsAccelCopy(&ret->kvm, &qemuCaps->kvm) < 0 || virQEMUCapsAccelCopy(&ret->tcg, &qemuCaps->tcg) < 0) - goto error; + return NULL; ret->gicCapabilities = g_new0(virGICCapability, qemuCaps->ngicCapabilities); ret->ngicCapabilities = qemuCaps->ngicCapabilities; @@ -1990,13 +1990,9 @@ virQEMUCaps *virQEMUCapsNewCopy(virQEMUCaps *qemuCaps) if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST) && virQEMUCapsSEVInfoCopy(&ret->sevCapabilities, qemuCaps->sevCapabilities) < 0) - goto error; + return NULL; - return ret; - - error: - virObjectUnref(ret); - return NULL; + return g_steal_pointer(&ret); } -- 2.31.1
Where easily possible, declare variables with g_auto to reduce the amount of calls in cleanup sections. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index fecd332604..4dc2a9805d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2980,10 +2980,10 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps, virDomainVirtType virtType) { const char *model = virtType == VIR_DOMAIN_VIRT_KVM ? "host" : "max"; - qemuMonitorCPUModelInfo *modelInfo = NULL; - qemuMonitorCPUModelInfo *nonMigratable = NULL; - GHashTable *hash = NULL; - virCPUDef *cpu; + g_autoptr(qemuMonitorCPUModelInfo) modelInfo = NULL; + g_autoptr(qemuMonitorCPUModelInfo) nonMigratable = NULL; + g_autoptr(GHashTable) hash = NULL; + g_autoptr(virCPUDef) cpu = NULL; qemuMonitorCPUModelExpansionType type; bool fail_no_props = true; int ret = -1; @@ -3061,10 +3061,6 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps, ret = 0; cleanup: - virHashFree(hash); - qemuMonitorCPUModelInfoFree(nonMigratable); - qemuMonitorCPUModelInfoFree(modelInfo); - virCPUDefFree(cpu); return ret; } @@ -3084,7 +3080,7 @@ virQEMUCapsGetCPUFeatures(virQEMUCaps *qemuCaps, char ***features) { qemuMonitorCPUModelInfo *modelInfo; - char **list; + g_auto(GStrv) list = NULL; size_t i; size_t n; int ret = -1; @@ -3113,7 +3109,6 @@ virQEMUCapsGetCPUFeatures(virQEMUCaps *qemuCaps, else ret = 0; - g_strfreev(list); return ret; } @@ -3499,7 +3494,7 @@ virQEMUCapsGetCPUModelX86Data(virQEMUCaps *qemuCaps, unsigned long long sigFamily = 0; unsigned long long sigModel = 0; unsigned long long sigStepping = 0; - virCPUData *data = NULL; + g_autoptr(virCPUData) data = NULL; virCPUData *ret = NULL; size_t i; @@ -3547,7 +3542,6 @@ virQEMUCapsGetCPUModelX86Data(virQEMUCaps *qemuCaps, ret = g_steal_pointer(&data); cleanup: - virCPUDataFree(data); return ret; } @@ -3565,7 +3559,7 @@ virQEMUCapsInitCPUModelX86(virQEMUCaps *qemuCaps, bool migratable) { g_autoptr(virDomainCapsCPUModels) cpuModels = NULL; - virCPUData *data = NULL; + g_autoptr(virCPUData) data = NULL; int ret = -1; if (!model) @@ -3582,7 +3576,6 @@ virQEMUCapsInitCPUModelX86(virQEMUCaps *qemuCaps, ret = 0; cleanup: - virCPUDataFree(data); return ret; } @@ -3769,9 +3762,9 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, { char *str = NULL; xmlNodePtr hostCPUNode; - xmlNodePtr *nodes = NULL; + g_autofree xmlNodePtr *nodes = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) - qemuMonitorCPUModelInfo *hostCPU = NULL; + g_autoptr(qemuMonitorCPUModelInfo) hostCPU = NULL; g_autofree char *xpath = g_strdup_printf("./hostCPU[@type='%s']", typeStr); int ret = -1; size_t i; @@ -3881,8 +3874,6 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, cleanup: VIR_FREE(str); - VIR_FREE(nodes); - qemuMonitorCPUModelInfoFree(hostCPU); return ret; } @@ -4655,7 +4646,7 @@ virQEMUCapsSaveFile(void *data, void *privData G_GNUC_UNUSED) { virQEMUCaps *qemuCaps = data; - char *xml = NULL; + g_autofree char *xml = NULL; int ret = -1; xml = virQEMUCapsFormatCache(qemuCaps); @@ -4674,7 +4665,6 @@ virQEMUCapsSaveFile(void *data, ret = 0; cleanup: - VIR_FREE(xml); return ret; } @@ -5189,7 +5179,7 @@ virQEMUCapsProbeQMPSchemaCapabilities(virQEMUCaps *qemuCaps, { struct virQEMUCapsStringFlags *entry; virJSONValue *schemareply; - GHashTable *schema = NULL; + g_autoptr(GHashTable) schema = NULL; size_t i; if (!(schemareply = qemuMonitorQueryQMPSchema(mon))) @@ -5214,7 +5204,6 @@ virQEMUCapsProbeQMPSchemaCapabilities(virQEMUCaps *qemuCaps, virQEMUCapsSet(qemuCaps, entry->flag); } - virHashFree(schema); return 0; } @@ -5575,7 +5564,7 @@ virQEMUCapsCacheNew(const char *libDir, uid_t runUid, gid_t runGid) { - char *capsCacheDir = NULL; + g_autofree char *capsCacheDir = NULL; virFileCache *cache = NULL; virQEMUCapsCachePriv *priv = NULL; struct utsname uts; @@ -5603,7 +5592,6 @@ virQEMUCapsCacheNew(const char *libDir, priv->kernelVersion = g_strdup_printf("%s %s", uts.release, uts.version); cleanup: - VIR_FREE(capsCacheDir); return cache; error: -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 44 ++++++++++++------------------------ 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4dc2a9805d..286d34ae54 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2986,7 +2986,6 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps, g_autoptr(virCPUDef) cpu = NULL; qemuMonitorCPUModelExpansionType type; bool fail_no_props = true; - int ret = -1; if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) return 0; @@ -3017,14 +3016,14 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps, if (qemuMonitorGetCPUModelExpansion(mon, type, cpu, true, fail_no_props, &modelInfo) < 0) - goto cleanup; + return -1; /* Try to check migratability of each feature. */ if (modelInfo && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_MIGRATABLE) && qemuMonitorGetCPUModelExpansion(mon, type, cpu, false, fail_no_props, &nonMigratable) < 0) - goto cleanup; + return -1; if (nonMigratable) { qemuMonitorCPUProperty *prop; @@ -3036,7 +3035,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps, for (i = 0; i < modelInfo->nprops; i++) { prop = modelInfo->props + i; if (virHashAddEntry(hash, prop->name, prop) < 0) - goto cleanup; + return -1; } for (i = 0; i < nonMigratable->nprops; i++) { @@ -3058,11 +3057,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps, } accel->hostCPU.info = g_steal_pointer(&modelInfo); - ret = 0; - - cleanup: - - return ret; + return 0; } @@ -3495,11 +3490,10 @@ virQEMUCapsGetCPUModelX86Data(virQEMUCaps *qemuCaps, unsigned long long sigModel = 0; unsigned long long sigStepping = 0; g_autoptr(virCPUData) data = NULL; - virCPUData *ret = NULL; size_t i; if (!(data = virCPUDataNew(VIR_ARCH_X86_64))) - goto cleanup; + return NULL; for (i = 0; i < model->nprops; i++) { qemuMonitorCPUProperty *prop = model->props + i; @@ -3512,14 +3506,14 @@ virQEMUCapsGetCPUModelX86Data(virQEMUCaps *qemuCaps, continue; if (virCPUDataAddFeature(data, name) < 0) - goto cleanup; + return NULL; break; case QEMU_MONITOR_CPU_PROPERTY_STRING: if (STREQ(name, "vendor") && virCPUx86DataSetVendor(data, prop->value.string) < 0) - goto cleanup; + return NULL; break; case QEMU_MONITOR_CPU_PROPERTY_NUMBER: @@ -3537,12 +3531,9 @@ virQEMUCapsGetCPUModelX86Data(virQEMUCaps *qemuCaps, } if (virCPUx86DataSetSignature(data, sigFamily, sigModel, sigStepping) < 0) - goto cleanup; + return NULL; - ret = g_steal_pointer(&data); - - cleanup: - return ret; + return g_steal_pointer(&data); } @@ -3560,23 +3551,19 @@ virQEMUCapsInitCPUModelX86(virQEMUCaps *qemuCaps, { g_autoptr(virDomainCapsCPUModels) cpuModels = NULL; g_autoptr(virCPUData) data = NULL; - int ret = -1; if (!model) return 1; if (!(data = virQEMUCapsGetCPUModelX86Data(qemuCaps, model, migratable))) - goto cleanup; + return -1; cpuModels = virQEMUCapsGetCPUModels(qemuCaps, type, NULL, NULL); if (cpuDecode(cpu, data, cpuModels) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - return ret; + return 0; } @@ -4647,7 +4634,6 @@ virQEMUCapsSaveFile(void *data, { virQEMUCaps *qemuCaps = data; g_autofree char *xml = NULL; - int ret = -1; xml = virQEMUCapsFormatCache(qemuCaps); @@ -4655,7 +4641,7 @@ virQEMUCapsSaveFile(void *data, virReportSystemError(errno, _("Failed to save '%s' for '%s'"), filename, qemuCaps->binary); - goto cleanup; + return -1; } VIR_DEBUG("Saved caps '%s' for '%s' with (%lld, %lld)", @@ -4663,9 +4649,7 @@ virQEMUCapsSaveFile(void *data, (long long)qemuCaps->ctime, (long long)qemuCaps->libvirtCtime); - ret = 0; - cleanup: - return ret; + return 0; } -- 2.31.1
On Mon, Aug 23, 2021 at 4:38 PM Ján Tomko <jtomko@redhat.com> wrote:
This is not a cover letter.
Ján Tomko (6): qemu: refactor virQEMUCapsNewForBinaryInternal qemu: refactor virQEMUCapsLoadFile qemu: refactor virQEMUCapsInit qemu: refactor virQEMUCapsNewCopy qemu: capabilities: use g_auto qemu: capabilities: remove pointless labels
src/qemu/qemu_capabilities.c | 134 ++++++++++++----------------------- 1 file changed, 45 insertions(+), 89 deletions(-)
-- 2.31.1
Reviewed-by: Kristina Hanicova <khanicov@redhat.com> Kristina
On Mon, Aug 23, 2021 at 05:26:43PM +0200, Kristina Hanicova wrote:
On Mon, Aug 23, 2021 at 4:38 PM Ján Tomko <jtomko@redhat.com> wrote:
This is not a cover letter.
Ján Tomko (6): qemu: refactor virQEMUCapsNewForBinaryInternal qemu: refactor virQEMUCapsLoadFile qemu: refactor virQEMUCapsInit qemu: refactor virQEMUCapsNewCopy qemu: capabilities: use g_auto qemu: capabilities: remove pointless labels
src/qemu/qemu_capabilities.c | 134 ++++++++++++----------------------- 1 file changed, 45 insertions(+), 89 deletions(-)
-- 2.31.1
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
On 8/24/21 3:22 PM, Martin Kletzander wrote:
On Mon, Aug 23, 2021 at 05:26:43PM +0200, Kristina Hanicova wrote:
On Mon, Aug 23, 2021 at 4:38 PM Ján Tomko <jtomko@redhat.com> wrote:
This is not a cover letter.
Ján Tomko (6): qemu: refactor virQEMUCapsNewForBinaryInternal qemu: refactor virQEMUCapsLoadFile qemu: refactor virQEMUCapsInit qemu: refactor virQEMUCapsNewCopy qemu: capabilities: use g_auto qemu: capabilities: remove pointless labels
src/qemu/qemu_capabilities.c | 134 ++++++++++++----------------------- 1 file changed, 45 insertions(+), 89 deletions(-)
-- 2.31.1
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On a Tuesday in 2021, Michal Prívozník wrote:
On 8/24/21 3:22 PM, Martin Kletzander wrote:
On Mon, Aug 23, 2021 at 05:26:43PM +0200, Kristina Hanicova wrote:
On Mon, Aug 23, 2021 at 4:38 PM Ján Tomko <jtomko@redhat.com> wrote:
This is not a cover letter.
Ján Tomko (6): qemu: refactor virQEMUCapsNewForBinaryInternal qemu: refactor virQEMUCapsLoadFile qemu: refactor virQEMUCapsInit qemu: refactor virQEMUCapsNewCopy qemu: capabilities: use g_auto qemu: capabilities: remove pointless labels
src/qemu/qemu_capabilities.c | 134 ++++++++++++----------------------- 1 file changed, 45 insertions(+), 89 deletions(-)
-- 2.31.1
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (4)
-
Ján Tomko -
Kristina Hanicova -
Martin Kletzander -
Michal Prívozník