[libvirt] [PATCH v4 0/5] qemu_driver: GLib powered cleanup

changes from v3: - only use the new Glib macros in all patches - different patch split, as suggested by Jano in v3 review - patch 5 (new): g_strdup_printf conversion changes from v2: - rebased with newer master (67e72053c1) - added an extra patch to convert the existing VIR_AUTO* macros to g_auto* ones, all at once, to avoid the case where a method will have both VIR_AUTO* and g_auto* macros at the same time. changes from v1: - addressed review concerns made by Erik - added more cleanups, as suggested by Erik v3: https://www.redhat.com/archives/libvir-list/2019-October/msg00918.html v2: https://www.redhat.com/archives/libvir-list/2019-September/msg01452.html v1: https://www.redhat.com/archives/libvir-list/2019-September/msg00719.html Daniel Henrique Barboza (5): qemu_driver: use g_auto* in some functions qemu_driver: use g_autoptr() when possible qemu_driver: use g_autofree when possible qemu_driver: remove unused 'cleanup' labels after g_auto*() changes qemu_driver: use g_strdup_printf src/qemu/qemu_driver.c | 721 ++++++++++++++--------------------------- 1 file changed, 242 insertions(+), 479 deletions(-) -- 2.21.0

This patch changes qemuDomainSnapshotLoad, qemuDomainCheckpointLoad and qemuStateInitialize to use g_autoptr() and g_autofree, cleaning up some virObjectUnref() and VIR_FREE() calls on each. The reason this is being sent separately is because these are not trivial search/replace cases. In all these functions some strings declarations are moved inside local loops, where they are in fact used, allowing us to erase VIR_FREE() calls that were made inside the loop and in 'cleanup' labels. Following patches with tackle more trivial cases of g_auto* usage in all qemu_driver.c file. Suggested-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 51 +++++++++++++----------------------------- 1 file changed, 15 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 86d7647628..3772c71a51 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -386,11 +386,9 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, void *data) { char *baseDir = (char *)data; - char *snapDir = NULL; + g_autofree char *snapDir = NULL; DIR *dir = NULL; struct dirent *entry; - char *xmlStr; - char *fullpath; virDomainSnapshotDefPtr def = NULL; virDomainMomentObjPtr snap = NULL; virDomainMomentObjPtr current = NULL; @@ -399,7 +397,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, VIR_DOMAIN_SNAPSHOT_PARSE_DISKS | VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL); int ret = -1; - virCapsPtr caps = NULL; + g_autoptr(virCaps) caps = NULL; int direrr; qemuDomainObjPrivatePtr priv; @@ -425,6 +423,9 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, goto cleanup; while ((direrr = virDirRead(dir, &entry, NULL)) > 0) { + g_autofree char *xmlStr = NULL; + g_autofree char *fullpath = NULL; + /* NB: ignoring errors, so one malformed config doesn't kill the whole process */ VIR_INFO("Loading snapshot file '%s'", entry->d_name); @@ -440,7 +441,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, virReportSystemError(errno, _("Failed to read snapshot file %s"), fullpath); - VIR_FREE(fullpath); continue; } @@ -453,8 +453,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to parse snapshot XML from file '%s'"), fullpath); - VIR_FREE(fullpath); - VIR_FREE(xmlStr); continue; } @@ -468,9 +466,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, vm->def->name); current = snap; } - - VIR_FREE(fullpath); - VIR_FREE(xmlStr); } if (direrr < 0) virReportError(VIR_ERR_INTERNAL_ERROR, @@ -497,8 +492,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, ret = 0; cleanup: VIR_DIR_CLOSE(dir); - VIR_FREE(snapDir); - virObjectUnref(caps); virObjectUnlock(vm); return ret; } @@ -509,17 +502,15 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, void *data) { char *baseDir = (char *)data; - char *chkDir = NULL; + g_autofree char *chkDir = NULL; DIR *dir = NULL; struct dirent *entry; - char *xmlStr; - char *fullpath; virDomainCheckpointDefPtr def = NULL; virDomainMomentObjPtr chk = NULL; virDomainMomentObjPtr current = NULL; unsigned int flags = VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE; int ret = -1; - virCapsPtr caps = NULL; + g_autoptr(virCaps) caps = NULL; int direrr; qemuDomainObjPrivatePtr priv; @@ -544,6 +535,9 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, goto cleanup; while ((direrr = virDirRead(dir, &entry, NULL)) > 0) { + g_autofree char *xmlStr = NULL; + g_autofree char *fullpath = NULL; + /* NB: ignoring errors, so one malformed config doesn't kill the whole process */ VIR_INFO("Loading checkpoint file '%s'", entry->d_name); @@ -559,7 +553,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, virReportSystemError(errno, _("Failed to read checkpoint file %s"), fullpath); - VIR_FREE(fullpath); continue; } @@ -572,8 +565,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to parse checkpoint XML from file '%s'"), fullpath); - VIR_FREE(fullpath); - VIR_FREE(xmlStr); virObjectUnref(def); continue; } @@ -581,9 +572,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, chk = virDomainCheckpointAssignDef(vm->checkpoints, def); if (chk == NULL) virObjectUnref(def); - - VIR_FREE(fullpath); - VIR_FREE(xmlStr); } if (direrr < 0) virReportError(VIR_ERR_INTERNAL_ERROR, @@ -608,8 +596,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, ret = 0; cleanup: VIR_DIR_CLOSE(dir); - VIR_FREE(chkDir); - virObjectUnref(caps); virObjectUnlock(vm); return ret; } @@ -667,12 +653,11 @@ qemuStateInitialize(bool privileged, virStateInhibitCallback callback, void *opaque) { - char *driverConf = NULL; + g_autofree char *driverConf = NULL; virQEMUDriverConfigPtr cfg; uid_t run_uid = -1; gid_t run_gid = -1; - char *hugepagePath = NULL; - char *memoryBackingPath = NULL; + g_autofree char *memoryBackingPath = NULL; bool autostart = true; size_t i; @@ -713,7 +698,6 @@ qemuStateInitialize(bool privileged, if (virQEMUDriverConfigLoadFile(cfg, driverConf, privileged) < 0) goto error; - VIR_FREE(driverConf); if (virQEMUDriverConfigValidate(cfg) < 0) goto error; @@ -837,7 +821,7 @@ qemuStateInitialize(bool privileged, goto error; if (privileged) { - char *channeldir; + g_autofree char *channeldir = NULL; if (chown(cfg->libDir, cfg->user, cfg->group) < 0) { virReportSystemError(errno, @@ -890,10 +874,8 @@ qemuStateInitialize(bool privileged, _("unable to set ownership of '%s' to %d:%d"), channeldir, (int)cfg->user, (int)cfg->group); - VIR_FREE(channeldir); goto error; } - VIR_FREE(channeldir); if (chown(cfg->channelTargetDir, cfg->user, cfg->group) < 0) { virReportSystemError(errno, _("unable to set ownership of '%s' to %d:%d"), @@ -944,6 +926,8 @@ qemuStateInitialize(bool privileged, * it, since we can't assume the root mount point has permissions that * will let our spawned QEMU instances use it. */ for (i = 0; i < cfg->nhugetlbfs; i++) { + g_autofree char *hugepagePath = NULL; + hugepagePath = qemuGetBaseHugepagePath(&cfg->hugetlbfs[i]); if (!hugepagePath) @@ -959,7 +943,6 @@ qemuStateInitialize(bool privileged, virFileUpdatePerm(cfg->hugetlbfs[i].mnt_dir, 0, S_IXGRP | S_IXOTH) < 0) goto error; - VIR_FREE(hugepagePath); } if (qemuGetMemoryBackingBasePath(cfg, &memoryBackingPath) < 0) @@ -976,7 +959,6 @@ qemuStateInitialize(bool privileged, virFileUpdatePerm(memoryBackingPath, 0, S_IXGRP | S_IXOTH) < 0) goto error; - VIR_FREE(memoryBackingPath); if (!(qemu_driver->closeCallbacks = virCloseCallbacksNew())) goto error; @@ -1045,9 +1027,6 @@ qemuStateInitialize(bool privileged, return VIR_DRV_STATE_INIT_COMPLETE; error: - VIR_FREE(driverConf); - VIR_FREE(hugepagePath); - VIR_FREE(memoryBackingPath); qemuStateCleanup(); return VIR_DRV_STATE_INIT_ERROR; } -- 2.21.0

Several pointer types can be auto-unref for the great majority of the uses made in qemu_driver, sparing us a virObjectUnref() call. This patch uses g_autoptr() in the following pointer types inside qemu_driver.c, whenever possible: - qemuBlockJobDataPtr - virCapsPtr - virConnect - virDomainCapsPtr - virNetworkPtr - virQEMUDriverConfigPtr Suggested-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 271 +++++++++++++---------------------------- 1 file changed, 87 insertions(+), 184 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3772c71a51..dc342734b1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -202,7 +202,7 @@ qemuAutostartDomain(virDomainObjPtr vm, { virQEMUDriverPtr driver = opaque; int flags = 0; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); int ret = -1; if (cfg->autoStartBypassCache) @@ -234,7 +234,6 @@ qemuAutostartDomain(virDomainObjPtr vm, ret = 0; cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -308,7 +307,7 @@ qemuSecurityInit(virQEMUDriverPtr driver) char **names; virSecurityManagerPtr mgr = NULL; virSecurityManagerPtr stack = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); unsigned int flags = 0; if (cfg->securityDefaultConfined) @@ -368,7 +367,6 @@ qemuSecurityInit(virQEMUDriverPtr driver) } driver->securityManager = stack; - virObjectUnref(cfg); return 0; error: @@ -376,7 +374,6 @@ qemuSecurityInit(virQEMUDriverPtr driver) _("Failed to initialize security drivers")); virObjectUnref(stack); virObjectUnref(mgr); - virObjectUnref(cfg); return -1; } @@ -1053,8 +1050,8 @@ static void qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque) static int qemuStateReload(void) { - virQEMUDriverConfigPtr cfg = NULL; - virCapsPtr caps = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; + g_autoptr(virCaps) caps = NULL; if (!qemu_driver) return 0; @@ -1069,8 +1066,6 @@ qemuStateReload(void) caps, qemu_driver->xmlopt, qemuNotifyLoadDomain, qemu_driver); cleanup: - virObjectUnref(cfg); - virObjectUnref(caps); return 0; } @@ -1085,13 +1080,13 @@ static int qemuStateStop(void) { int ret = -1; - virConnectPtr conn; + g_autoptr(virConnect) conn = NULL; int numDomains = 0; size_t i; int state; virDomainPtr *domains = NULL; unsigned int *flags = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(qemu_driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver); if (!(conn = virConnectOpen(cfg->uri))) goto cleanup; @@ -1129,8 +1124,6 @@ qemuStateStop(void) VIR_FREE(domains); } VIR_FREE(flags); - virObjectUnref(conn); - virObjectUnref(cfg); return ret; } @@ -1188,7 +1181,7 @@ qemuStateCleanup(void) static int qemuConnectURIProbe(char **uri) { - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; int ret = -1; if (qemu_driver == NULL) @@ -1200,7 +1193,6 @@ qemuConnectURIProbe(char **uri) ret = 0; cleanup: - virObjectUnref(cfg); return ret; } @@ -1344,8 +1336,8 @@ qemuConnectGetMaxVcpus(virConnectPtr conn G_GNUC_UNUSED, const char *type) static char *qemuConnectGetCapabilities(virConnectPtr conn) { virQEMUDriverPtr driver = conn->privateData; - virCapsPtr caps = NULL; char *xml = NULL; + g_autoptr(virCaps) caps = NULL; if (virConnectGetCapabilitiesEnsureACL(conn) < 0) return NULL; @@ -1354,7 +1346,6 @@ static char *qemuConnectGetCapabilities(virConnectPtr conn) { goto cleanup; xml = virCapabilitiesFormatXML(caps); - virObjectUnref(caps); cleanup: @@ -1706,7 +1697,7 @@ static int qemuConnectGetVersion(virConnectPtr conn, unsigned long *version) virQEMUDriverPtr driver = conn->privateData; int ret = -1; unsigned int qemuVersion = 0; - virCapsPtr caps = NULL; + g_autoptr(virCaps) caps = NULL; if (virConnectGetVersionEnsureACL(conn) < 0) return -1; @@ -1723,7 +1714,6 @@ static int qemuConnectGetVersion(virConnectPtr conn, unsigned long *version) ret = 0; cleanup: - virObjectUnref(caps); return ret; } @@ -1777,7 +1767,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, virObjectEventPtr event = NULL; virObjectEventPtr event2 = NULL; unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD; - virCapsPtr caps = NULL; + g_autoptr(virCaps) caps = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; @@ -1852,7 +1842,6 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, virDomainObjEndAPI(&vm); virObjectEventStateQueue(driver->domainEventState, event); virObjectEventStateQueue(driver->domainEventState, event2); - virObjectUnref(caps); virNWFilterUnlockFilterUpdates(); return dom; } @@ -1866,7 +1855,7 @@ static int qemuDomainSuspend(virDomainPtr dom) qemuDomainObjPrivatePtr priv; virDomainPausedReason reason; int state; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; if (!(vm = qemuDomainObjFromDomain(dom))) return -1; @@ -1909,7 +1898,6 @@ static int qemuDomainSuspend(virDomainPtr dom) cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -1921,7 +1909,7 @@ static int qemuDomainResume(virDomainPtr dom) int ret = -1; int state; int reason; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; if (!(vm = qemuDomainObjFromDomain(dom))) return -1; @@ -1967,7 +1955,6 @@ static int qemuDomainResume(virDomainPtr dom) cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -2348,7 +2335,7 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, virDomainDefPtr def; virDomainDefPtr persistentDef; int ret = -1, r; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -2453,7 +2440,6 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -2476,7 +2462,7 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period, virDomainDefPtr def; virDomainDefPtr persistentDef; int ret = -1, r; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -2539,7 +2525,6 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -3052,14 +3037,12 @@ qemuOpenFile(virQEMUDriverPtr driver, bool *needUnlink) { int ret = -1; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); uid_t user = cfg->user; gid_t group = cfg->group; bool dynamicOwnership = cfg->dynamicOwnership; virSecurityLabelDefPtr seclabel; - virObjectUnref(cfg); - /* TODO: Take imagelabel into account? */ if (vm && (seclabel = virDomainDefGetSecurityLabelDef(vm->def, "dac")) != NULL && @@ -3236,7 +3219,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, unsigned int flags, qemuDomainAsyncJob asyncJob) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); bool needUnlink = false; int ret = -1; int fd = -1; @@ -3299,7 +3282,6 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, if (qemuFileWrapperFDClose(vm, wrapperFd) < 0) ret = -1; virFileWrapperFdFree(wrapperFd); - virObjectUnref(cfg); if (ret < 0 && needUnlink) unlink(path); @@ -3323,7 +3305,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, int ret = -1; virObjectEventPtr event = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; - virCapsPtr caps; + g_autoptr(virCaps) caps = NULL; virQEMUSaveDataPtr data = NULL; qemuDomainSaveCookiePtr cookie = NULL; @@ -3438,7 +3420,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, VIR_FREE(xml); virQEMUSaveDataFree(data); virObjectEventStateQueue(driver->domainEventState, event); - virObjectUnref(caps); return ret; } @@ -3527,7 +3508,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, char *compressedpath = NULL; int ret = -1; virDomainObjPtr vm = NULL; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | VIR_DOMAIN_SAVE_RUNNING | @@ -3554,7 +3535,6 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, cleanup: virDomainObjEndAPI(&vm); VIR_FREE(compressedpath); - virObjectUnref(cfg); return ret; } @@ -3568,14 +3548,11 @@ static char * qemuDomainManagedSavePath(virQEMUDriverPtr driver, virDomainObjPtr vm) { char *ret; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - if (virAsprintf(&ret, "%s/%s.save", cfg->saveDir, vm->def->name) < 0) { - virObjectUnref(cfg); + if (virAsprintf(&ret, "%s/%s.save", cfg->saveDir, vm->def->name) < 0) return NULL; - } - virObjectUnref(cfg); return ret; } @@ -3583,7 +3560,7 @@ static int qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; int compressed; char *compressedpath = NULL; virDomainObjPtr vm; @@ -3629,7 +3606,6 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) virDomainObjEndAPI(&vm); VIR_FREE(name); VIR_FREE(compressedpath); - virObjectUnref(cfg); return ret; } @@ -3827,7 +3803,7 @@ doCoreDump(virQEMUDriverPtr driver, int directFlag = 0; unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING; const char *memory_dump_format = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); char *compressedpath = NULL; /* We reuse "save" flag for "dump" here. Then, we can support the same @@ -3909,7 +3885,6 @@ doCoreDump(virQEMUDriverPtr driver, if (ret != 0) unlink(path); VIR_FREE(compressedpath); - virObjectUnref(cfg); return ret; } @@ -4041,7 +4016,7 @@ qemuDomainScreenshot(virDomainPtr dom, const char *videoAlias = NULL; char *ret = NULL; bool unlink_tmp = false; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virCheckFlags(0, NULL); @@ -4133,7 +4108,6 @@ qemuDomainScreenshot(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -4146,7 +4120,7 @@ getAutoDumpPath(virQEMUDriverPtr driver, char timestr[100]; struct tm time_info; time_t curtime = time(NULL); - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; if (!domname) return NULL; @@ -4161,7 +4135,6 @@ getAutoDumpPath(virQEMUDriverPtr driver, domname, timestr)); - virObjectUnref(cfg); VIR_FREE(domname); return dumpfile; } @@ -4172,7 +4145,7 @@ processWatchdogEvent(virQEMUDriverPtr driver, int action) { int ret; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); char *dumpfile = getAutoDumpPath(driver, vm); unsigned int flags = VIR_DUMP_MEMORY_ONLY; @@ -4213,7 +4186,6 @@ processWatchdogEvent(virQEMUDriverPtr driver, qemuDomainObjEndAsyncJob(driver, vm); cleanup: - virObjectUnref(cfg); VIR_FREE(dumpfile); } @@ -4223,7 +4195,7 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver, unsigned int flags) { int ret = -1; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); char *dumpfile = getAutoDumpPath(driver, vm); if (!dumpfile) @@ -4236,7 +4208,6 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver, "%s", _("Dump failed")); cleanup: VIR_FREE(dumpfile); - virObjectUnref(cfg); return ret; } @@ -4265,13 +4236,13 @@ processGuestPanicEvent(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; virObjectEventPtr event = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); bool removeInactive = false; unsigned long flags = VIR_DUMP_MEMORY_ONLY; if (qemuDomainObjBeginAsyncJob(driver, vm, QEMU_ASYNC_JOB_DUMP, VIR_DOMAIN_JOB_OPERATION_DUMP, flags) < 0) - goto cleanup; + return; if (!virDomainObjIsActive(vm)) { VIR_DEBUG("Ignoring GUEST_PANICKED event from inactive domain %s", @@ -4338,9 +4309,6 @@ processGuestPanicEvent(virQEMUDriverPtr driver, qemuDomainObjEndAsyncJob(driver, vm); if (removeInactive) qemuDomainRemoveInactiveJob(driver, vm); - - cleanup: - virObjectUnref(cfg); } @@ -4349,14 +4317,14 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *devAlias) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virDomainDeviceDef dev; VIR_DEBUG("Removing device %s from domain %p %s", devAlias, vm, vm->def->name); if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; + return; if (!virDomainObjIsActive(vm)) { VIR_DEBUG("Domain is not running"); @@ -4379,9 +4347,6 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver, endjob: qemuDomainObjEndJob(driver, vm); - - cleanup: - virObjectUnref(cfg); } @@ -4586,7 +4551,7 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *devAlias) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDeviceDef dev; virDomainNetDefPtr def; @@ -4685,7 +4650,6 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver, cleanup: virNetDevRxFilterFree(hostFilter); virNetDevRxFilterFree(guestFilter); - virObjectUnref(cfg); } @@ -4695,7 +4659,7 @@ processSerialChangedEvent(virQEMUDriverPtr driver, const char *devAlias, bool connected) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virDomainChrDeviceState newstate; virObjectEventPtr event = NULL; virDomainDeviceDef dev; @@ -4728,7 +4692,7 @@ processSerialChangedEvent(virQEMUDriverPtr driver, } if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; + return; if (!virDomainObjIsActive(vm)) { VIR_DEBUG("Domain is not running"); @@ -4769,9 +4733,6 @@ processSerialChangedEvent(virQEMUDriverPtr driver, endjob: qemuDomainObjEndJob(driver, vm); - - cleanup: - virObjectUnref(cfg); } @@ -5044,7 +5005,7 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver, virDomainDefPtr persistentDef, unsigned int nvcpus) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); unsigned int topologycpus; int ret = -1; @@ -5082,7 +5043,6 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver, ret = 0; cleanup: - virObjectUnref(cfg); return ret; } @@ -5248,7 +5208,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, int ret = -1; virBitmapPtr pcpumap = NULL; virDomainVcpuDefPtr vcpuinfo = NULL; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -5305,7 +5265,6 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); virBitmapFree(pcpumap); - virObjectUnref(cfg); return ret; } @@ -5368,7 +5327,7 @@ qemuDomainPinEmulator(virDomainPtr dom, int ret = -1; qemuDomainObjPrivatePtr priv; virBitmapPtr pcpumap = NULL; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virObjectEventPtr event = NULL; char *str = NULL; virTypedParameterPtr eventParams = NULL; @@ -5462,7 +5421,6 @@ qemuDomainPinEmulator(virDomainPtr dom, VIR_FREE(str); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -5833,7 +5791,7 @@ qemuDomainPinIOThread(virDomainPtr dom, { int ret = -1; virQEMUDriverPtr driver = dom->conn->privateData; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virDomainObjPtr vm; virDomainDefPtr def; virDomainDefPtr persistentDef; @@ -5957,7 +5915,6 @@ qemuDomainPinIOThread(virDomainPtr dom, VIR_FREE(str); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -6306,7 +6263,7 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver, virDomainIOThreadAction action, unsigned int flags) { - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; qemuDomainObjPrivatePtr priv; virDomainDefPtr def; virDomainDefPtr persistentDef; @@ -6406,7 +6363,6 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver, qemuDomainObjEndJob(driver, vm); cleanup: - virObjectUnref(cfg); return ret; } @@ -6634,7 +6590,7 @@ static int qemuNodeGetSecurityModel(virConnectPtr conn, virQEMUDriverPtr driver = conn->privateData; char *p; int ret = 0; - virCapsPtr caps = NULL; + g_autoptr(virCaps) caps = NULL; memset(secmodel, 0, sizeof(*secmodel)); @@ -6670,7 +6626,6 @@ static int qemuNodeGetSecurityModel(virConnectPtr conn, strcpy(secmodel->doi, p); cleanup: - virObjectUnref(caps); return ret; } @@ -6692,7 +6647,7 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver, virDomainDefPtr ret = NULL; virDomainDefPtr newdef_migr = NULL; virDomainDefPtr newdef = NULL; - virCapsPtr caps = NULL; + g_autoptr(virCaps) caps = NULL; if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; @@ -6730,7 +6685,6 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver, } cleanup: - virObjectUnref(caps); virDomainDefFree(newdef); virDomainDefFree(newdef_migr); @@ -6770,7 +6724,7 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, virQEMUSaveHeaderPtr header; virDomainDefPtr def = NULL; int oflags = open_write ? O_RDWR : O_RDONLY; - virCapsPtr caps = NULL; + g_autoptr(virCaps) caps = NULL; size_t xml_len; size_t cookie_len; @@ -6890,7 +6844,6 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, *ret_data = data; cleanup: - virObjectUnref(caps); return fd; error: @@ -6917,7 +6870,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, int intermediatefd = -1; virCommandPtr cmd = NULL; char *errbuf = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virQEMUSaveHeaderPtr header = &data->header; qemuDomainSaveCookiePtr cookie = NULL; @@ -7039,7 +6992,6 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, VIR_FREE(errbuf); if (qemuSecurityRestoreSavedStateLabel(driver, vm, path) < 0) VIR_WARN("failed to restore save state label on %s", path); - virObjectUnref(cfg); return ret; } @@ -7464,13 +7416,11 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, virCommandPtr cmd = NULL; char *ret = NULL; size_t i; - virQEMUDriverConfigPtr cfg; - virCapsPtr caps = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virCaps) caps = NULL; virCheckFlags(0, NULL); - cfg = virQEMUDriverGetConfig(driver); - if (virConnectDomainXMLToNativeEnsureACL(conn) < 0) goto cleanup; @@ -7529,8 +7479,6 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, cleanup: virCommandFree(cmd); virObjectUnref(vm); - virObjectUnref(caps); - virObjectUnref(cfg); return ret; } @@ -7717,8 +7665,8 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; virObjectEventPtr event = NULL; - virQEMUDriverConfigPtr cfg; - virCapsPtr caps = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virCaps) caps = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; @@ -7727,8 +7675,6 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, if (flags & VIR_DOMAIN_DEFINE_VALIDATE) parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; - cfg = virQEMUDriverGetConfig(driver); - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; @@ -7784,8 +7730,6 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, virDomainDefFree(def); virDomainObjEndAPI(&vm); virObjectEventStateQueue(driver->domainEventState, event); - virObjectUnref(caps); - virObjectUnref(cfg); return dom; } @@ -7806,7 +7750,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, int ret = -1; int nsnapshots; int ncheckpoints; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; g_autofree char *nvram_path = NULL; virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | @@ -7934,7 +7878,6 @@ qemuDomainUndefineFlags(virDomainPtr dom, VIR_FREE(name); virDomainObjEndAPI(&vm); virObjectEventStateQueue(driver->domainEventState, event); - virObjectUnref(cfg); return ret; } @@ -8754,11 +8697,11 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm, { qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr vmdef = NULL; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virDomainDeviceDefPtr devConf = NULL; virDomainDeviceDefPtr devLive = NULL; int ret = -1; - virCapsPtr caps = NULL; + g_autoptr(virCaps) caps = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; @@ -8839,8 +8782,6 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm, virDomainDefFree(vmdef); virDomainDeviceDefFree(devConf); virDomainDeviceDefFree(devLive); - virObjectUnref(cfg); - virObjectUnref(caps); return ret; } @@ -8900,8 +8841,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; int ret = -1; - virQEMUDriverConfigPtr cfg = NULL; - virCapsPtr caps = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; + g_autoptr(virCaps) caps = NULL; unsigned int parse_flags = 0; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -9000,8 +8941,6 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, virDomainDeviceDefFree(dev_copy); virDomainDeviceDefFree(dev); virDomainObjEndAPI(&vm); - virObjectUnref(caps); - virObjectUnref(cfg); virNWFilterUnlockFilterUpdates(); return ret; } @@ -9013,8 +8952,8 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, unsigned int flags) { qemuDomainObjPrivatePtr priv = vm->privateData; - virCapsPtr caps = NULL; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virCaps) caps = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; virDomainDefPtr vmdef = NULL; @@ -9092,8 +9031,6 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, ret = 0; cleanup: - virObjectUnref(caps); - virObjectUnref(cfg); if (dev != dev_copy) virDomainDeviceDefFree(dev_copy); virDomainDeviceDefFree(dev); @@ -9109,8 +9046,8 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver, unsigned int flags) { qemuDomainObjPrivatePtr priv = vm->privateData; - virCapsPtr caps = NULL; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virCaps) caps = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainDefPtr vmdef = NULL; @@ -9171,8 +9108,6 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver, ret = 0; cleanup: virDomainDefFree(vmdef); - virObjectUnref(cfg); - virObjectUnref(caps); return ret; } @@ -9280,7 +9215,7 @@ static int qemuDomainSetAutostart(virDomainPtr dom, virDomainObjPtr vm; char *configFile = NULL, *autostartLink = NULL; int ret = -1; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; if (!(vm = qemuDomainObjFromDomain(dom))) return -1; @@ -9345,7 +9280,6 @@ static int qemuDomainSetAutostart(virDomainPtr dom, VIR_FREE(configFile); VIR_FREE(autostartLink); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -9592,7 +9526,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, virDomainDefPtr def; virDomainDefPtr persistentDef; int ret = -1; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -9792,7 +9726,6 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -9904,7 +9837,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, bool set_swap_hard_limit = false; bool set_hard_limit = false; bool set_soft_limit = false; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; int rc; int ret = -1; qemuDomainObjPrivatePtr priv; @@ -10029,7 +9962,6 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -10194,7 +10126,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virDomainDefPtr persistentDef; virDomainObjPtr vm = NULL; int ret = -1; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; qemuDomainObjPrivatePtr priv; virBitmapPtr nodeset = NULL; virDomainNumatuneMemMode config_mode; @@ -10306,7 +10238,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom, cleanup: virBitmapFree(nodeset); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -10411,7 +10342,7 @@ qemuDomainSetPerfEvents(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; size_t i; virDomainObjPtr vm = NULL; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; qemuDomainObjPrivatePtr priv; virDomainDefPtr def; virDomainDefPtr persistentDef; @@ -10503,7 +10434,6 @@ qemuDomainSetPerfEvents(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -10690,8 +10620,8 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, long long value_l; int ret = -1; int rc; - virQEMUDriverConfigPtr cfg = NULL; - virCapsPtr caps = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; + g_autoptr(virCaps) caps = NULL; qemuDomainObjPrivatePtr priv; virObjectEventPtr event = NULL; virTypedParameterPtr eventParams = NULL; @@ -10983,8 +10913,6 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, virDomainObjEndAPI(&vm); if (eventNparams) virTypedParamsFree(eventParams, eventNparams); - virObjectUnref(caps); - virObjectUnref(cfg); return ret; } #undef SCHED_RANGE_CHECK @@ -11649,7 +11577,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, int ret = -1; virDomainNetDefPtr net = NULL, persistentNet = NULL; virNetDevBandwidthPtr bandwidth = NULL, newBandwidth = NULL; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; bool inboundSpecified = false, outboundSpecified = false; int actualType; bool qosSupported = true; @@ -11844,7 +11772,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, virNetDevBandwidthFree(bandwidth); virNetDevBandwidthFree(newBandwidth); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -12106,7 +12033,7 @@ qemuDomainMemoryPeek(virDomainPtr dom, char *tmp = NULL; int fd = -1, ret = -1; qemuDomainObjPrivatePtr priv; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virCheckFlags(VIR_MEMORY_VIRTUAL | VIR_MEMORY_PHYSICAL, -1); @@ -12177,7 +12104,6 @@ qemuDomainMemoryPeek(virDomainPtr dom, unlink(tmp); VIR_FREE(tmp); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -12394,7 +12320,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; virDomainDiskDefPtr disk; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; qemuBlockStatsPtr entry = NULL; virCheckFlags(0, -1); @@ -12483,7 +12409,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom, cleanup: VIR_FREE(entry); virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -12955,7 +12880,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, unsigned int flags) { virQEMUDriverPtr driver = dconn->privateData; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; @@ -13029,7 +12954,6 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, VIR_FREE(migrate_disks); VIR_FREE(origname); virDomainDefFree(def); - virObjectUnref(cfg); return ret; } @@ -13601,7 +13525,7 @@ qemuConnectCompareCPU(virConnectPtr conn, { virQEMUDriverPtr driver = conn->privateData; int ret = VIR_CPU_COMPARE_ERROR; - virCapsPtr caps = NULL; + g_autoptr(virCaps) caps = NULL; bool failIncompatible; virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE, @@ -13619,7 +13543,6 @@ qemuConnectCompareCPU(virConnectPtr conn, xmlDesc, failIncompatible); cleanup: - virObjectUnref(caps); return ret; } @@ -13675,7 +13598,7 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn, int ret = VIR_CPU_COMPARE_ERROR; virQEMUDriverPtr driver = conn->privateData; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - virQEMUCapsPtr qemuCaps = NULL; + g_autoptr(virQEMUCaps) qemuCaps = NULL; bool failIncompatible; virCPUDefPtr hvCPU; virCPUDefPtr cpu = NULL; @@ -13729,7 +13652,6 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn, cleanup: virCPUDefFree(cpu); - virObjectUnref(qemuCaps); return ret; } @@ -13889,7 +13811,7 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, virQEMUDriverPtr driver = conn->privateData; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virCPUDefPtr *cpus = NULL; - virQEMUCapsPtr qemuCaps = NULL; + g_autoptr(virQEMUCaps) qemuCaps = NULL; virArch arch; virDomainVirtType virttype; virDomainCapsCPUModelsPtr cpuModels; @@ -13968,7 +13890,6 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, cleanup: virCPUDefListFree(cpus); virCPUDefFree(cpu); - virObjectUnref(qemuCaps); virStringListFree(features); return cpustr; @@ -14753,7 +14674,7 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, virCommandPtr cmd = NULL; const char *qemuImgPath; virBitmapPtr created = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); int ret = -1; virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap); @@ -14841,7 +14762,6 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, } } virBitmapFree(created); - virObjectUnref(cfg); return ret; } @@ -15943,8 +15863,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; bool align_match = true; - virQEMUDriverConfigPtr cfg = NULL; - virCapsPtr caps = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; + g_autoptr(virCaps) caps = NULL; qemuDomainObjPrivatePtr priv; virDomainSnapshotState state; g_autoptr(virDomainSnapshotDef) def = NULL; @@ -16216,8 +16136,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, cleanup: virDomainObjEndAPI(&vm); VIR_FREE(xml); - virObjectUnref(caps); - virObjectUnref(cfg); return snapshot; } @@ -16627,8 +16545,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, int rc; virDomainDefPtr config = NULL; virDomainDefPtr inactiveConfig = NULL; - virQEMUDriverConfigPtr cfg = NULL; - virCapsPtr caps = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; + g_autoptr(virCaps) caps = NULL; bool was_stopped = false; qemuDomainSaveCookiePtr cookie; virCPUDefPtr origCPU = NULL; @@ -17045,8 +16963,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virObjectEventStateQueue(driver->domainEventState, event); virObjectEventStateQueue(driver->domainEventState, event2); virDomainObjEndAPI(&vm); - virObjectUnref(caps); - virObjectUnref(cfg); virNWFilterUnlockFilterUpdates(); virCPUDefFree(origCPU); virDomainDefFree(config); @@ -17107,7 +17023,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, virQEMUMomentReparent rep; bool metadata_only = !!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY); int external = 0; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY | @@ -17199,7 +17115,6 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -17779,10 +17694,10 @@ qemuDomainBlockJobAbort(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainDiskDefPtr disk = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); bool pivot = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT); bool async = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC); - qemuBlockJobDataPtr job = NULL; + g_autoptr(qemuBlockJobData) job = NULL; virDomainObjPtr vm; qemuDomainObjPrivatePtr priv = NULL; bool blockdev = false; @@ -17883,8 +17798,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - virObjectUnref(job); - virObjectUnref(cfg); virDomainObjEndAPI(&vm); return ret; } @@ -19144,7 +19057,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, bool supportMaxOptions = true; bool supportGroupNameOption = true; bool supportMaxLengthOptions = true; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virObjectEventPtr event = NULL; virTypedParameterPtr eventParams = NULL; int eventNparams = 0; @@ -19465,7 +19378,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, virDomainObjEndAPI(&vm); if (eventNparams) virTypedParamsFree(eventParams, eventNparams); - virObjectUnref(cfg); return ret; } @@ -19715,8 +19627,8 @@ qemuDomainSetMetadata(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - virQEMUDriverConfigPtr cfg = NULL; - virCapsPtr caps = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; + g_autoptr(virCaps) caps = NULL; int ret = -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -19750,8 +19662,6 @@ qemuDomainSetMetadata(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(caps); - virObjectUnref(cfg); return ret; } @@ -20559,10 +20469,10 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, { char *ret = NULL; virQEMUDriverPtr driver = conn->privateData; - virQEMUCapsPtr qemuCaps = NULL; + g_autoptr(virQEMUCaps) qemuCaps = NULL; virArch arch; virDomainVirtType virttype; - virDomainCapsPtr domCaps = NULL; + g_autoptr(virDomainCaps) domCaps = NULL; virCheckFlags(0, ret); @@ -20585,8 +20495,6 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, ret = virDomainCapsFormat(domCaps); cleanup: - virObjectUnref(domCaps); - virObjectUnref(qemuCaps); return ret; } @@ -21338,7 +21246,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, virHashTablePtr nodestats = NULL; virJSONValuePtr nodedata = NULL; qemuDomainObjPrivatePtr priv = dom->privateData; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); bool fetchnodedata = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_NAMED_BLOCK_NODES) && !blockdev; @@ -21396,7 +21304,6 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, virHashFree(stats); virHashFree(nodestats); virJSONValueFree(nodedata); - virObjectUnref(cfg); return ret; } @@ -21831,7 +21738,7 @@ qemuGetDHCPInterfaces(virDomainPtr dom, int n_leases = 0; size_t i, j; size_t ifaces_count = 0; - virNetworkPtr network = NULL; + g_autoptr(virNetwork) network = NULL; char macaddr[VIR_MAC_STRING_BUFLEN]; virDomainInterfacePtr iface = NULL; virNetworkDHCPLeasePtr *leases = NULL; @@ -21899,7 +21806,6 @@ qemuGetDHCPInterfaces(virDomainPtr dom, rv = ifaces_count; cleanup: - virObjectUnref(network); if (leases) { for (i = 0; i < n_leases; i++) virNetworkDHCPLeaseFree(leases[i]); @@ -22035,7 +21941,7 @@ qemuDomainRenameCallback(virDomainObjPtr vm, void *opaque) { virQEMUDriverPtr driver = opaque; - virQEMUDriverConfigPtr cfg = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; virObjectEventPtr event_new = NULL; virObjectEventPtr event_old = NULL; int ret = -1; @@ -22124,7 +22030,6 @@ qemuDomainRenameCallback(virDomainObjPtr vm, VIR_FREE(new_dom_name); virObjectEventStateQueue(driver->domainEventState, event_old); virObjectEventStateQueue(driver->domainEventState, event_new); - virObjectUnref(cfg); return ret; rollback: @@ -22587,7 +22492,7 @@ qemuDomainSetLifecycleAction(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv; virDomainObjPtr vm = NULL; virDomainDefPtr def = NULL; @@ -22644,7 +22549,6 @@ qemuDomainSetLifecycleAction(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } @@ -22696,8 +22600,8 @@ qemuNodeGetSEVInfo(virConnectPtr conn, unsigned int flags) { virQEMUDriverPtr driver = conn->privateData; - virQEMUCapsPtr qemucaps = NULL; int ret = -1; + g_autoptr(virQEMUCaps) qemucaps = NULL; if (virNodeGetSevInfoEnsureACL(conn) < 0) return ret; @@ -22719,7 +22623,6 @@ qemuNodeGetSEVInfo(virConnectPtr conn, ret = 0; cleanup: - virObjectUnref(qemucaps); return ret; } -- 2.21.0

String and other scalar pointers an be auto-unref, sparing us a VIR_FREE() call. This patch uses g_autofree whenever possible with strings and other scalar pointer types. Suggested-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 203 ++++++++++++++--------------------------- 1 file changed, 68 insertions(+), 135 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dc342734b1..d824992c16 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1085,7 +1085,7 @@ qemuStateStop(void) size_t i; int state; virDomainPtr *domains = NULL; - unsigned int *flags = NULL; + g_autofree unsigned int *flags = NULL; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver); if (!(conn = virConnectOpen(cfg->uri))) @@ -1123,7 +1123,6 @@ qemuStateStop(void) virObjectUnref(domains[i]); VIR_FREE(domains); } - VIR_FREE(flags); return ret; } @@ -1357,8 +1356,8 @@ static int qemuGetSchedInfo(unsigned long long *cpuWait, pid_t pid, pid_t tid) { - char *proc = NULL; - char *data = NULL; + g_autofree char *proc = NULL; + g_autofree char *data = NULL; char **lines = NULL; size_t i; int ret = -1; @@ -1422,8 +1421,6 @@ qemuGetSchedInfo(unsigned long long *cpuWait, ret = 0; cleanup: - VIR_FREE(data); - VIR_FREE(proc); virStringListFree(lines); return ret; } @@ -1433,7 +1430,7 @@ static int qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, pid_t pid, int tid) { - char *proc; + g_autofree char *proc = NULL; FILE *pidinfo; unsigned long long usertime = 0, systime = 0; long rss = 0; @@ -1450,7 +1447,6 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, return -1; pidinfo = fopen(proc, "r"); - VIR_FREE(proc); /* See 'man proc' for information about what all these fields are. We're * only interested in a very few of them */ @@ -2908,7 +2904,7 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data, size_t cookie_len = 0; int ret = -1; size_t zerosLen = 0; - char *zeros = NULL; + g_autofree char *zeros = NULL; xml_len = strlen(data->xml) + 1; if (data->cookie) @@ -2965,7 +2961,6 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data, ret = 0; cleanup: - VIR_FREE(zeros); return ret; } @@ -3300,7 +3295,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, int compressed, const char *compressedpath, const char *xmlin, unsigned int flags) { - char *xml = NULL; + g_autofree char *xml = NULL; bool was_running = false; int ret = -1; virObjectEventPtr event = NULL; @@ -3381,7 +3376,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, if (!(data = virQEMUSaveDataNew(xml, cookie, was_running, compressed, driver->xmlopt))) goto endjob; - xml = NULL; ret = qemuDomainSaveMemory(driver, vm, path, data, compressedpath, flags, QEMU_ASYNC_JOB_SAVE); @@ -3417,7 +3411,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, cleanup: virObjectUnref(cookie); - VIR_FREE(xml); virQEMUSaveDataFree(data); virObjectEventStateQueue(driver->domainEventState, event); return ret; @@ -3505,7 +3498,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, { virQEMUDriverPtr driver = dom->conn->privateData; int compressed; - char *compressedpath = NULL; + g_autofree char *compressedpath = NULL; int ret = -1; virDomainObjPtr vm = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; @@ -3534,7 +3527,6 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, cleanup: virDomainObjEndAPI(&vm); - VIR_FREE(compressedpath); return ret; } @@ -3562,9 +3554,9 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) virQEMUDriverPtr driver = dom->conn->privateData; g_autoptr(virQEMUDriverConfig) cfg = NULL; int compressed; - char *compressedpath = NULL; + g_autofree char *compressedpath = NULL; virDomainObjPtr vm; - char *name = NULL; + g_autofree char *name = NULL; int ret = -1; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | @@ -3604,8 +3596,6 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) cleanup: virDomainObjEndAPI(&vm); - VIR_FREE(name); - VIR_FREE(compressedpath); return ret; } @@ -3615,7 +3605,7 @@ qemuDomainManagedSaveLoad(virDomainObjPtr vm, void *opaque) { virQEMUDriverPtr driver = opaque; - char *name; + g_autofree char *name = NULL; int ret = -1; virObjectLock(vm); @@ -3628,7 +3618,6 @@ qemuDomainManagedSaveLoad(virDomainObjPtr vm, ret = 0; cleanup: virObjectUnlock(vm); - VIR_FREE(name); return ret; } @@ -3660,7 +3649,7 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; - char *name = NULL; + g_autofree char *name = NULL; virCheckFlags(0, -1); @@ -3684,7 +3673,6 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) ret = 0; cleanup: - VIR_FREE(name); virDomainObjEndAPI(&vm); return ret; } @@ -3804,7 +3792,7 @@ doCoreDump(virQEMUDriverPtr driver, unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING; const char *memory_dump_format = NULL; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - char *compressedpath = NULL; + g_autofree char *compressedpath = NULL; /* We reuse "save" flag for "dump" here. Then, we can support the same * format in "save" and "dump". This path doesn't need the compression @@ -3884,7 +3872,6 @@ doCoreDump(virQEMUDriverPtr driver, virFileWrapperFdFree(wrapperFd); if (ret != 0) unlink(path); - VIR_FREE(compressedpath); return ret; } @@ -4010,7 +3997,7 @@ qemuDomainScreenshot(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; - char *tmp = NULL; + g_autofree char *tmp = NULL; int tmp_fd = -1; size_t i; const char *videoAlias = NULL; @@ -4102,7 +4089,6 @@ qemuDomainScreenshot(virDomainPtr dom, VIR_FORCE_CLOSE(tmp_fd); if (unlink_tmp) unlink(tmp); - VIR_FREE(tmp); qemuDomainObjEndJob(driver, vm); @@ -4116,7 +4102,7 @@ getAutoDumpPath(virQEMUDriverPtr driver, virDomainObjPtr vm) { char *dumpfile = NULL; - char *domname = virDomainDefGetShortName(vm->def); + g_autofree char *domname = virDomainDefGetShortName(vm->def); char timestr[100]; struct tm time_info; time_t curtime = time(NULL); @@ -4135,7 +4121,6 @@ getAutoDumpPath(virQEMUDriverPtr driver, domname, timestr)); - VIR_FREE(domname); return dumpfile; } @@ -4146,11 +4131,11 @@ processWatchdogEvent(virQEMUDriverPtr driver, { int ret; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - char *dumpfile = getAutoDumpPath(driver, vm); + g_autofree char *dumpfile = getAutoDumpPath(driver, vm); unsigned int flags = VIR_DUMP_MEMORY_ONLY; if (!dumpfile) - goto cleanup; + return; switch (action) { case VIR_DOMAIN_WATCHDOG_ACTION_DUMP: @@ -4158,7 +4143,7 @@ processWatchdogEvent(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_DUMP, VIR_DOMAIN_JOB_OPERATION_DUMP, flags) < 0) { - goto cleanup; + return; } if (virDomainObjCheckActive(vm) < 0) @@ -4179,14 +4164,11 @@ processWatchdogEvent(virQEMUDriverPtr driver, "%s", _("Resuming after dump failed")); break; default: - goto cleanup; + return; } endjob: qemuDomainObjEndAsyncJob(driver, vm); - - cleanup: - VIR_FREE(dumpfile); } static int @@ -4196,7 +4178,7 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver, { int ret = -1; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - char *dumpfile = getAutoDumpPath(driver, vm); + g_autofree char *dumpfile = getAutoDumpPath(driver, vm); if (!dumpfile) goto cleanup; @@ -4207,7 +4189,6 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver, virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Dump failed")); cleanup: - VIR_FREE(dumpfile); return ret; } @@ -4217,14 +4198,11 @@ qemuProcessGuestPanicEventInfo(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuMonitorEventPanicInfoPtr info) { - char *msg = qemuMonitorGuestPanicEventInfoFormatMsg(info); - char *timestamp = virTimeStringNow(); + g_autofree char *msg = qemuMonitorGuestPanicEventInfoFormatMsg(info); + g_autofree char *timestamp = virTimeStringNow(); if (msg && timestamp) qemuDomainLogAppendMessage(driver, vm, "%s: panic %s\n", timestamp, msg); - - VIR_FREE(timestamp); - VIR_FREE(msg); } @@ -5124,7 +5102,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, virDomainVcpuDefPtr vcpuinfo; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup_vcpu = NULL; - char *str = NULL; + g_autofree char *str = NULL; virObjectEventPtr event = NULL; char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = ""; virTypedParameterPtr eventParams = NULL; @@ -5188,7 +5166,6 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, cleanup: virBitmapFree(tmpmap); virCgroupFree(&cgroup_vcpu); - VIR_FREE(str); virObjectEventStateQueue(driver->domainEventState, event); return ret; } @@ -5329,7 +5306,7 @@ qemuDomainPinEmulator(virDomainPtr dom, virBitmapPtr pcpumap = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; virObjectEventPtr event = NULL; - char *str = NULL; + g_autofree char *str = NULL; virTypedParameterPtr eventParams = NULL; int eventNparams = 0; int eventMaxparams = 0; @@ -5418,7 +5395,6 @@ qemuDomainPinEmulator(virDomainPtr dom, if (cgroup_emulator) virCgroupFree(&cgroup_emulator); virObjectEventStateQueue(driver->domainEventState, event); - VIR_FREE(str); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); return ret; @@ -5800,7 +5776,7 @@ qemuDomainPinIOThread(virDomainPtr dom, virCgroupPtr cgroup_iothread = NULL; virObjectEventPtr event = NULL; char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = ""; - char *str = NULL; + g_autofree char *str = NULL; virTypedParameterPtr eventParams = NULL; int eventNparams = 0; int eventMaxparams = 0; @@ -5912,7 +5888,6 @@ qemuDomainPinIOThread(virDomainPtr dom, if (cgroup_iothread) virCgroupFree(&cgroup_iothread); virObjectEventStateQueue(driver->domainEventState, event); - VIR_FREE(str); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); return ret; @@ -5924,7 +5899,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, unsigned int iothread_id) { qemuDomainObjPrivatePtr priv = vm->privateData; - char *alias = NULL; + g_autofree char *alias = NULL; size_t idx; int ret = -1; unsigned int orig_niothreads = vm->def->niothreadids; @@ -6000,7 +5975,6 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, } virDomainAuditIOThread(vm, orig_niothreads, new_niothreads, "update", ret == 0); - VIR_FREE(alias); virJSONValueFree(props); return ret; @@ -6045,7 +6019,7 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; size_t idx; - char *alias = NULL; + g_autofree char *alias = NULL; int rc = -1; int ret = -1; unsigned int orig_niothreads = vm->def->niothreadids; @@ -6094,7 +6068,6 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver, } virDomainAuditIOThread(vm, orig_niothreads, new_niothreads, "update", rc == 0); - VIR_FREE(alias); return ret; exit_monitor: @@ -6869,7 +6842,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, virObjectEventPtr event; int intermediatefd = -1; virCommandPtr cmd = NULL; - char *errbuf = NULL; + g_autofree char *errbuf = NULL; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virQEMUSaveHeaderPtr header = &data->header; qemuDomainSaveCookiePtr cookie = NULL; @@ -6989,7 +6962,6 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, cleanup: virObjectUnref(cookie); virCommandFree(cmd); - VIR_FREE(errbuf); if (qemuSecurityRestoreSavedStateLabel(driver, vm, path) < 0) VIR_WARN("failed to restore save state label on %s", path); return ret; @@ -7005,7 +6977,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, qemuDomainObjPrivatePtr priv = NULL; virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; - char *xmlout = NULL; + g_autofree char *xmlout = NULL; const char *newxml = dxml; int fd = -1; int ret = -1; @@ -7090,7 +7062,6 @@ qemuDomainRestoreFlags(virConnectPtr conn, ret = -1; virFileWrapperFdFree(wrapperFd); virQEMUSaveDataFree(data); - VIR_FREE(xmlout); if (vm && ret < 0) qemuDomainRemoveInactiveJob(driver, vm); virDomainObjEndAPI(&vm); @@ -7213,7 +7184,7 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - char *path = NULL; + g_autofree char *path = NULL; char *ret = NULL; virDomainDefPtr def = NULL; int fd = -1; @@ -7250,7 +7221,6 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) virDomainDefFree(def); VIR_FORCE_CLOSE(fd); virDomainObjEndAPI(&vm); - VIR_FREE(path); return ret; } @@ -7261,7 +7231,7 @@ qemuDomainManagedSaveDefineXML(virDomainPtr dom, const char *dxml, virQEMUDriverPtr driver = dom->conn->privateData; virConnectPtr conn = dom->conn; virDomainObjPtr vm; - char *path = NULL; + g_autofree char *path = NULL; int ret = -1; if (!(vm = qemuDomainObjFromDomain(dom))) @@ -7283,7 +7253,6 @@ qemuDomainManagedSaveDefineXML(virDomainPtr dom, const char *dxml, cleanup: virDomainObjEndAPI(&vm); - VIR_FREE(path); return ret; } @@ -7302,7 +7271,7 @@ qemuDomainObjRestore(virConnectPtr conn, qemuDomainObjPrivatePtr priv = vm->privateData; int fd = -1; int ret = -1; - char *xmlout = NULL; + g_autofree char *xmlout = NULL; virQEMUSaveDataPtr data = NULL; virFileWrapperFdPtr wrapperFd = NULL; @@ -7359,7 +7328,6 @@ qemuDomainObjRestore(virConnectPtr conn, cleanup: virQEMUSaveDataFree(data); - VIR_FREE(xmlout); virDomainDefFree(def); VIR_FORCE_CLOSE(fd); if (virFileWrapperFdClose(wrapperFd) < 0) @@ -7524,7 +7492,7 @@ qemuDomainObjStart(virConnectPtr conn, qemuDomainAsyncJob asyncJob) { int ret = -1; - char *managed_save; + g_autofree char *managed_save = NULL; bool start_paused = (flags & VIR_DOMAIN_START_PAUSED) != 0; bool autodestroy = (flags & VIR_DOMAIN_START_AUTODESTROY) != 0; bool bypass_cache = (flags & VIR_DOMAIN_START_BYPASS_CACHE) != 0; @@ -7598,7 +7566,6 @@ qemuDomainObjStart(virConnectPtr conn, } cleanup: - VIR_FREE(managed_save); return ret; } @@ -7746,7 +7713,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; virObjectEventPtr event = NULL; - char *name = NULL; + g_autofree char *name = NULL; int ret = -1; int nsnapshots; int ncheckpoints; @@ -7875,7 +7842,6 @@ qemuDomainUndefineFlags(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - VIR_FREE(name); virDomainObjEndAPI(&vm); virObjectEventStateQueue(driver->domainEventState, event); return ret; @@ -9213,7 +9179,8 @@ static int qemuDomainSetAutostart(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - char *configFile = NULL, *autostartLink = NULL; + g_autofree char *configFile = NULL; + g_autofree char *autostartLink = NULL; int ret = -1; g_autoptr(virQEMUDriverConfig) cfg = NULL; @@ -9277,8 +9244,6 @@ static int qemuDomainSetAutostart(virDomainPtr dom, ret = 0; cleanup: - VIR_FREE(configFile); - VIR_FREE(autostartLink); virDomainObjEndAPI(&vm); return ret; } @@ -10058,7 +10023,7 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, { virCgroupPtr cgroup_temp = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; - char *nodeset_str = NULL; + g_autofree char *nodeset_str = NULL; virDomainNumatuneMemMode mode; size_t i = 0; int ret = -1; @@ -10108,7 +10073,6 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, ret = 0; cleanup: - VIR_FREE(nodeset_str); virCgroupFree(&cgroup_temp); return ret; @@ -10251,7 +10215,7 @@ qemuDomainGetNumaParameters(virDomainPtr dom, virDomainObjPtr vm = NULL; virDomainNumatuneMemMode tmpmode = VIR_DOMAIN_NUMATUNE_MEM_STRICT; qemuDomainObjPrivatePtr priv; - char *nodeset = NULL; + g_autofree char *nodeset = NULL; int ret = -1; virDomainDefPtr def = NULL; bool live = false; @@ -10315,7 +10279,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, ret = 0; cleanup: - VIR_FREE(nodeset); virDomainObjEndAPI(&vm); return ret; } @@ -11182,7 +11145,7 @@ qemuDomainBlockResize(virDomainPtr dom, virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; int ret = -1; - char *device = NULL; + g_autofree char *device = NULL; const char *nodename = NULL; virDomainDiskDefPtr disk = NULL; @@ -11254,7 +11217,6 @@ qemuDomainBlockResize(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - VIR_FREE(device); virDomainObjEndAPI(&vm); return ret; } @@ -11963,7 +11925,7 @@ qemuDomainBlockPeek(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainDiskDefPtr disk = NULL; virDomainObjPtr vm; - char *tmpbuf = NULL; + g_autofree char *tmpbuf = NULL; ssize_t nread; int ret = -1; @@ -12018,7 +11980,6 @@ qemuDomainBlockPeek(virDomainPtr dom, if (disk) virStorageFileDeinit(disk->src); virDomainObjEndAPI(&vm); - VIR_FREE(tmpbuf); return ret; } @@ -12030,7 +11991,7 @@ qemuDomainMemoryPeek(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - char *tmp = NULL; + g_autofree char *tmp = NULL; int fd = -1, ret = -1; qemuDomainObjPrivatePtr priv; g_autoptr(virQEMUDriverConfig) cfg = NULL; @@ -12102,7 +12063,6 @@ qemuDomainMemoryPeek(virDomainPtr dom, VIR_FORCE_CLOSE(fd); if (tmp) unlink(tmp); - VIR_FREE(tmp); virDomainObjEndAPI(&vm); return ret; } @@ -12268,7 +12228,7 @@ qemuStorageLimitsRefresh(virQEMUDriverPtr driver, int ret = -1; int fd = -1; struct stat sb; - char *buf = NULL; + g_autofree char *buf = NULL; ssize_t len; if ((rc = qemuDomainStorageOpenStat(driver, cfg, vm, src, &fd, &sb, @@ -12304,7 +12264,6 @@ qemuStorageLimitsRefresh(virQEMUDriverPtr driver, ret = 1; cleanup: - VIR_FREE(buf); qemuDomainStorageCloseStat(src, &fd); return ret; } @@ -12525,7 +12484,7 @@ qemuDomainMigratePrepareTunnel(virConnectPtr dconn, { virQEMUDriverPtr driver = dconn->privateData; virDomainDefPtr def = NULL; - char *origname = NULL; + g_autofree char *origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -12560,7 +12519,6 @@ qemuDomainMigratePrepareTunnel(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -12582,7 +12540,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, { virQEMUDriverPtr driver = dconn->privateData; virDomainDefPtr def = NULL; - char *origname = NULL; + g_autofree char *origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -12627,7 +12585,6 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -12828,7 +12785,7 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, { virQEMUDriverPtr driver = dconn->privateData; virDomainDefPtr def = NULL; - char *origname = NULL; + g_autofree char *origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -12863,7 +12820,6 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -12889,7 +12845,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, int nbdPort = 0; int nmigrate_disks; const char **migrate_disks = NULL; - char *origname = NULL; + g_autofree char *origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -12952,7 +12908,6 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); VIR_FREE(migrate_disks); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -12972,7 +12927,7 @@ qemuDomainMigratePrepareTunnel3(virConnectPtr dconn, { virQEMUDriverPtr driver = dconn->privateData; virDomainDefPtr def = NULL; - char *origname = NULL; + g_autofree char *origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -13001,7 +12956,6 @@ qemuDomainMigratePrepareTunnel3(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -13021,7 +12975,7 @@ qemuDomainMigratePrepareTunnel3Params(virConnectPtr dconn, virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; - char *origname = NULL; + g_autofree char *origname = NULL; qemuMigrationParamsPtr migParams = NULL; int ret = -1; @@ -13060,7 +13014,6 @@ qemuDomainMigratePrepareTunnel3Params(virConnectPtr dconn, cleanup: qemuMigrationParamsFree(migParams); - VIR_FREE(origname); virDomainDefFree(def); return ret; } @@ -13382,7 +13335,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, unsigned domain = 0, bus = 0, slot = 0, function = 0; int ret = -1; virNodeDeviceDefPtr def = NULL; - char *xml = NULL; + g_autofree char *xml = NULL; bool vfio = qemuHostdevHostSupportsPassthroughVFIO(); virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; @@ -13432,7 +13385,6 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, cleanup: virPCIDeviceFree(pci); virNodeDeviceDefFree(def); - VIR_FREE(xml); return ret; } @@ -13450,7 +13402,7 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) unsigned domain = 0, bus = 0, slot = 0, function = 0; int ret = -1; virNodeDeviceDefPtr def = NULL; - char *xml = NULL; + g_autofree char *xml = NULL; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; xml = virNodeDeviceGetXMLDesc(dev, 0); @@ -13476,7 +13428,6 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) virPCIDeviceFree(pci); cleanup: virNodeDeviceDefFree(def); - VIR_FREE(xml); return ret; } @@ -13488,7 +13439,7 @@ qemuNodeDeviceReset(virNodeDevicePtr dev) unsigned domain = 0, bus = 0, slot = 0, function = 0; int ret = -1; virNodeDeviceDefPtr def = NULL; - char *xml = NULL; + g_autofree char *xml = NULL; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; xml = virNodeDeviceGetXMLDesc(dev, 0); @@ -13514,7 +13465,6 @@ qemuNodeDeviceReset(virNodeDevicePtr dev) virPCIDeviceFree(pci); cleanup: virNodeDeviceDefFree(def); - VIR_FREE(xml); return ret; } @@ -15355,7 +15305,6 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; - char *backingStoreStr; virDomainDiskDefPtr persistdisk; g_autoptr(virStorageSource) terminator = NULL; bool supportsCreate; @@ -15401,13 +15350,13 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, * block commit still works */ if (reuse) { if (supportsBacking) { + g_autofree char *backingStoreStr = NULL; + if (virStorageFileGetBackingStoreStr(dd->src, &backingStoreStr) < 0) return -1; if (backingStoreStr != NULL) { if (virStorageIsRelative(backingStoreStr)) VIR_STEAL_PTR(dd->relPath, backingStoreStr); - else - VIR_FREE(backingStoreStr); } } } else { @@ -15854,7 +15803,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, { virQEMUDriverPtr driver = domain->conn->privateData; virDomainObjPtr vm = NULL; - char *xml = NULL; + g_autofree char *xml = NULL; virDomainMomentObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; virDomainMomentObjPtr current = NULL; @@ -16135,7 +16084,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, cleanup: virDomainObjEndAPI(&vm); - VIR_FREE(xml); return snapshot; } @@ -17922,7 +17870,7 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, virDomainDiskDefPtr disk; int ret = -1; virDomainObjPtr vm; - char *device = NULL; + g_autofree char *device = NULL; unsigned long long speed = bandwidth; virCheckFlags(VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES, -1); @@ -17967,7 +17915,6 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - VIR_FREE(device); virDomainObjEndAPI(&vm); return ret; @@ -19047,7 +18994,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainBlockIoTuneInfo info; - char *drivealias = NULL; + g_autofree char *drivealias = NULL; const char *qdevid = NULL; int ret = -1; size_t i; @@ -19374,7 +19321,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, cleanup: VIR_FREE(info.group_name); - VIR_FREE(drivealias); virDomainObjEndAPI(&vm); if (eventNparams) virTypedParamsFree(eventParams, eventNparams); @@ -19395,7 +19341,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainBlockIoTuneInfo reply = {0}; - char *drivealias = NULL; + g_autofree char *drivealias = NULL; const char *qdevid = NULL; int ret = -1; int maxparams; @@ -19532,7 +19478,6 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, cleanup: VIR_FREE(reply.group_name); - VIR_FREE(drivealias); virDomainObjEndAPI(&vm); return ret; } @@ -20812,7 +20757,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver, size_t i; int ret = -1; virVcpuInfoPtr cpuinfo = NULL; - unsigned long long *cpuwait = NULL; + g_autofree unsigned long long *cpuwait = NULL; if (virTypedParamListAddUInt(params, virDomainDefGetVcpus(dom->def), "vcpu.current") < 0) @@ -20877,7 +20822,6 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver, cleanup: VIR_FREE(cpuinfo); - VIR_FREE(cpuwait); return ret; } @@ -21159,7 +21103,6 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, bool blockdev) { - char *alias = NULL; virStorageSourcePtr n; const char *frontendalias; const char *backendalias; @@ -21181,6 +21124,8 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, } for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { + g_autofree char *alias = NULL; + if (blockdev) { frontendalias = QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName; backendalias = n->nodeformat; @@ -21218,7 +21163,6 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, params) < 0) goto cleanup; - VIR_FREE(alias); (*recordnr)++; if (!visitBacking) @@ -21228,7 +21172,6 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, ret = 0; cleanup: - VIR_FREE(alias); return ret; } @@ -21945,12 +21888,12 @@ qemuDomainRenameCallback(virDomainObjPtr vm, virObjectEventPtr event_new = NULL; virObjectEventPtr event_old = NULL; int ret = -1; - char *new_dom_name = NULL; - char *old_dom_name = NULL; - char *new_dom_cfg_file = NULL; - char *old_dom_cfg_file = NULL; - char *new_dom_autostart_link = NULL; - char *old_dom_autostart_link = NULL; + g_autofree char *new_dom_name = NULL; + g_autofree char *old_dom_name = NULL; + g_autofree char *new_dom_cfg_file = NULL; + g_autofree char *old_dom_cfg_file = NULL; + g_autofree char *new_dom_autostart_link = NULL; + g_autofree char *old_dom_autostart_link = NULL; virCheckFlags(0, ret); @@ -22022,12 +21965,6 @@ qemuDomainRenameCallback(virDomainObjPtr vm, ret = 0; cleanup: - VIR_FREE(old_dom_autostart_link); - VIR_FREE(new_dom_autostart_link); - VIR_FREE(old_dom_cfg_file); - VIR_FREE(new_dom_cfg_file); - VIR_FREE(old_dom_name); - VIR_FREE(new_dom_name); virObjectEventStateQueue(driver->domainEventState, event_old); virObjectEventStateQueue(driver->domainEventState, event_new); return ret; @@ -22132,7 +22069,7 @@ qemuDomainGetGuestVcpusParams(virTypedParameterPtr *params, virBitmapPtr vcpus = NULL; virBitmapPtr online = NULL; virBitmapPtr offlinable = NULL; - char *tmp = NULL; + g_autofree char *tmp = NULL; size_t i; int ret = -1; @@ -22161,7 +22098,6 @@ qemuDomainGetGuestVcpusParams(virTypedParameterPtr *params, goto cleanup; \ if (virTypedParamsAddString(&par, &npar, &maxpar, #name, tmp) < 0) \ goto cleanup; \ - VIR_FREE(tmp) ADD_BITMAP(vcpus); ADD_BITMAP(online); @@ -22175,7 +22111,6 @@ qemuDomainGetGuestVcpusParams(virTypedParameterPtr *params, ret = 0; cleanup: - VIR_FREE(tmp); virBitmapFree(vcpus); virBitmapFree(online); virBitmapFree(offlinable); @@ -22403,7 +22338,7 @@ qemuDomainSetBlockThreshold(virDomainPtr dom, qemuDomainObjPrivatePtr priv; virDomainObjPtr vm = NULL; virStorageSourcePtr src; - char *nodename = NULL; + g_autofree char *nodename = NULL; int rc; int ret = -1; @@ -22457,7 +22392,6 @@ qemuDomainSetBlockThreshold(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - VIR_FREE(nodename); virDomainObjEndAPI(&vm); return ret; } @@ -22636,7 +22570,7 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver, unsigned int flags) { int ret = -1; - char *tmp = NULL; + g_autofree char *tmp = NULL; int maxpar = 0; virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); @@ -22661,7 +22595,6 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver, ret = 0; endjob: - VIR_FREE(tmp); qemuDomainObjEndJob(driver, vm); return ret; } -- 2.21.0

On 10/16/19 10:54 PM, Daniel Henrique Barboza wrote:
String and other scalar pointers an be auto-unref, sparing us a VIR_FREE() call.
This patch uses g_autofree whenever possible with strings and other scalar pointer types.
Suggested-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 203 ++++++++++++++--------------------------- 1 file changed, 68 insertions(+), 135 deletions(-)
@@ -3300,7 +3295,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, int compressed, const char *compressedpath, const char *xmlin, unsigned int flags) { - char *xml = NULL; + g_autofree char *xml = NULL; bool was_running = false; int ret = -1; virObjectEventPtr event = NULL; @@ -3381,7 +3376,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, if (!(data = virQEMUSaveDataNew(xml, cookie, was_running, compressed, driver->xmlopt))) goto endjob; - xml = NULL;
No, this line has to be here. The point is that virQEMUSaveDataNew() takes ownership of @xml (in a very clumsy way though), so qemuDomainSaveInternal() has to refrain from freeing it. Setting it to NULL is how we achieve that.
ret = qemuDomainSaveMemory(driver, vm, path, data, compressedpath, flags, QEMU_ASYNC_JOB_SAVE);
Michal

On 10/17/19 9:10 AM, Michal Privoznik wrote:
On 10/16/19 10:54 PM, Daniel Henrique Barboza wrote:
String and other scalar pointers an be auto-unref, sparing us a VIR_FREE() call.
This patch uses g_autofree whenever possible with strings and other scalar pointer types.
Suggested-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 203 ++++++++++++++--------------------------- 1 file changed, 68 insertions(+), 135 deletions(-)
@@ -3300,7 +3295,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, int compressed, const char *compressedpath, const char *xmlin, unsigned int flags) { - char *xml = NULL; + g_autofree char *xml = NULL; bool was_running = false; int ret = -1; virObjectEventPtr event = NULL; @@ -3381,7 +3376,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, if (!(data = virQEMUSaveDataNew(xml, cookie, was_running, compressed, driver->xmlopt))) goto endjob; - xml = NULL;
No, this line has to be here. The point is that virQEMUSaveDataNew() takes ownership of @xml (in a very clumsy way though), so qemuDomainSaveInternal() has to refrain from freeing it. Setting it to NULL is how we achieve that.
Interesting. I'll keep an eye for this kind of usage next time. Thanks, DHB
ret = qemuDomainSaveMemory(driver, vm, path, data, compressedpath, flags, QEMU_ASYNC_JOB_SAVE);
Michal

The g_auto*() changes made by the previous patches made a lot of 'cleanup' labels obsolete. Let's remove them. Suggested-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 160 +++++++++++++++-------------------------- 1 file changed, 56 insertions(+), 104 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d824992c16..a263393626 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1057,7 +1057,7 @@ qemuStateReload(void) return 0; if (!(caps = virQEMUDriverGetCapabilities(qemu_driver, false))) - goto cleanup; + return 0; cfg = virQEMUDriverGetConfig(qemu_driver); virDomainObjListLoadAllConfigs(qemu_driver->domains, @@ -1065,7 +1065,6 @@ qemuStateReload(void) cfg->autostartDir, false, caps, qemu_driver->xmlopt, qemuNotifyLoadDomain, qemu_driver); - cleanup: return 0; } @@ -1181,18 +1180,15 @@ static int qemuConnectURIProbe(char **uri) { g_autoptr(virQEMUDriverConfig) cfg = NULL; - int ret = -1; if (qemu_driver == NULL) return 0; cfg = virQEMUDriverGetConfig(qemu_driver); if (VIR_STRDUP(*uri, cfg->uri) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - return ret; + return 0; } static virDrvOpenStatus qemuConnectOpen(virConnectPtr conn, @@ -1335,20 +1331,15 @@ qemuConnectGetMaxVcpus(virConnectPtr conn G_GNUC_UNUSED, const char *type) static char *qemuConnectGetCapabilities(virConnectPtr conn) { virQEMUDriverPtr driver = conn->privateData; - char *xml = NULL; g_autoptr(virCaps) caps = NULL; if (virConnectGetCapabilitiesEnsureACL(conn) < 0) return NULL; if (!(caps = virQEMUDriverGetCapabilities(driver, true))) - goto cleanup; - - xml = virCapabilitiesFormatXML(caps); - - cleanup: + return NULL; - return xml; + return virCapabilitiesFormatXML(caps); } @@ -1691,7 +1682,6 @@ static int qemuDomainIsUpdated(virDomainPtr dom) static int qemuConnectGetVersion(virConnectPtr conn, unsigned long *version) { virQEMUDriverPtr driver = conn->privateData; - int ret = -1; unsigned int qemuVersion = 0; g_autoptr(virCaps) caps = NULL; @@ -1699,18 +1689,15 @@ static int qemuConnectGetVersion(virConnectPtr conn, unsigned long *version) return -1; if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; + return -1; if (virQEMUCapsGetDefaultVersion(caps, driver->qemuCapsCache, &qemuVersion) < 0) - goto cleanup; + return -1; *version = qemuVersion; - ret = 0; - - cleanup: - return ret; + return 0; } @@ -2902,7 +2889,6 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data, size_t len; size_t xml_len; size_t cookie_len = 0; - int ret = -1; size_t zerosLen = 0; g_autofree char *zeros = NULL; @@ -2916,12 +2902,12 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data, if (len > header->data_len) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("new xml too large to fit in file")); - goto cleanup; + return -1; } zerosLen = header->data_len - len; if (VIR_ALLOC_N(zeros, zerosLen) < 0) - goto cleanup; + return -1; } else { header->data_len = len; } @@ -2933,14 +2919,14 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data, virReportSystemError(errno, _("failed to write header to domain save file '%s'"), path); - goto cleanup; + return -1; } if (safewrite(fd, data->xml, xml_len) != xml_len) { virReportSystemError(errno, _("failed to write domain xml to '%s'"), path); - goto cleanup; + return -1; } if (data->cookie && @@ -2948,20 +2934,17 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data, virReportSystemError(errno, _("failed to write cookie to '%s'"), path); - goto cleanup; + return -1; } if (safewrite(fd, zeros, zerosLen) != zerosLen) { virReportSystemError(errno, _("failed to write padding to '%s'"), path); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -3031,7 +3014,6 @@ qemuOpenFile(virQEMUDriverPtr driver, int oflags, bool *needUnlink) { - int ret = -1; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); uid_t user = cfg->user; gid_t group = cfg->group; @@ -3043,13 +3025,10 @@ qemuOpenFile(virQEMUDriverPtr driver, (seclabel = virDomainDefGetSecurityLabelDef(vm->def, "dac")) != NULL && seclabel->label != NULL && (virParseOwnershipIds(seclabel->label, &user, &group) < 0)) - goto cleanup; - - ret = qemuOpenFileAs(user, group, dynamicOwnership, - path, oflags, needUnlink); + return -1; - cleanup: - return ret; + return qemuOpenFileAs(user, group, dynamicOwnership, + path, oflags, needUnlink); } static int @@ -4181,14 +4160,13 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver, g_autofree char *dumpfile = getAutoDumpPath(driver, vm); if (!dumpfile) - goto cleanup; + return -1; flags |= cfg->autoDumpBypassCache ? VIR_DUMP_BYPASS_CACHE: 0; if ((ret = doCoreDump(driver, vm, dumpfile, flags, VIR_DOMAIN_CORE_DUMP_FORMAT_RAW)) < 0) virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Dump failed")); - cleanup: return ret; } @@ -4985,19 +4963,18 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver, { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); unsigned int topologycpus; - int ret = -1; if (def) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("maximum vcpu count of a live domain can't be modified")); - goto cleanup; + return -1; } if (virDomainNumaGetCPUCountTotal(persistentDef->numa) > nvcpus) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Number of CPUs in <numa> exceeds the desired " "maximum vcpu count")); - goto cleanup; + return -1; } if (virDomainDefGetVcpusTopology(persistentDef, &topologycpus) == 0 && @@ -5006,22 +4983,19 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver, * setting may be corrected via this API */ virReportError(VIR_ERR_INVALID_ARG, "%s", _("CPU topology doesn't match the desired vcpu count")); - goto cleanup; + return -1; } /* ordering information may become invalid, thus clear it */ virDomainDefVcpuOrderClear(persistentDef); if (virDomainDefSetVcpusMax(persistentDef, nvcpus, driver->xmlopt) < 0) - goto cleanup; + return -1; if (virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - return ret; + return 0; } @@ -6247,7 +6221,7 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver, priv = vm->privateData; if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; + return -1; if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto endjob; @@ -6335,7 +6309,6 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver, endjob: qemuDomainObjEndJob(driver, vm); - cleanup: return ret; } @@ -6562,29 +6535,27 @@ static int qemuNodeGetSecurityModel(virConnectPtr conn, { virQEMUDriverPtr driver = conn->privateData; char *p; - int ret = 0; g_autoptr(virCaps) caps = NULL; memset(secmodel, 0, sizeof(*secmodel)); if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; + return 0; if (virNodeGetSecurityModelEnsureACL(conn) < 0) - goto cleanup; + return 0; /* We treat no driver as success, but simply return no data in *secmodel */ if (caps->host.nsecModels == 0 || caps->host.secModels[0].model == NULL) - goto cleanup; + return 0; p = caps->host.secModels[0].model; if (strlen(p) >= VIR_SECURITY_MODEL_BUFLEN-1) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security model string exceeds max %d bytes"), VIR_SECURITY_MODEL_BUFLEN-1); - ret = -1; - goto cleanup; + return -1; } strcpy(secmodel->model, p); @@ -6593,13 +6564,11 @@ static int qemuNodeGetSecurityModel(virConnectPtr conn, virReportError(VIR_ERR_INTERNAL_ERROR, _("security DOI string exceeds max %d bytes"), VIR_SECURITY_DOI_BUFLEN-1); - ret = -1; - goto cleanup; + return -1; } strcpy(secmodel->doi, p); - cleanup: - return ret; + return 0; } @@ -7510,7 +7479,7 @@ qemuDomainObjStart(virConnectPtr conn, managed_save = qemuDomainManagedSavePath(driver, vm); if (!managed_save) - goto cleanup; + return ret; if (virFileExists(managed_save)) { if (force_boot) { @@ -7518,7 +7487,7 @@ qemuDomainObjStart(virConnectPtr conn, virReportSystemError(errno, _("cannot remove managed save file %s"), managed_save); - goto cleanup; + return ret; } vm->hasManagedSave = false; } else { @@ -7534,11 +7503,11 @@ qemuDomainObjStart(virConnectPtr conn, else vm->hasManagedSave = false; - goto cleanup; + return ret; } else if (ret < 0) { VIR_WARN("Unable to restore from managed state %s. " "Maybe the file is corrupted?", managed_save); - goto cleanup; + return ret; } else { VIR_WARN("Ignoring incomplete managed state %s", managed_save); priv->job.current->operation = op; @@ -7565,7 +7534,6 @@ qemuDomainObjStart(virConnectPtr conn, } } - cleanup: return ret; } @@ -13474,7 +13442,6 @@ qemuConnectCompareCPU(virConnectPtr conn, unsigned int flags) { virQEMUDriverPtr driver = conn->privateData; - int ret = VIR_CPU_COMPARE_ERROR; g_autoptr(virCaps) caps = NULL; bool failIncompatible; @@ -13482,18 +13449,15 @@ qemuConnectCompareCPU(virConnectPtr conn, VIR_CPU_COMPARE_ERROR); if (virConnectCompareCPUEnsureACL(conn) < 0) - goto cleanup; + return VIR_CPU_COMPARE_ERROR; failIncompatible = !!(flags & VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE); if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; - - ret = virCPUCompareXML(caps->host.arch, caps->host.cpu, - xmlDesc, failIncompatible); + return VIR_CPU_COMPARE_ERROR; - cleanup: - return ret; + return virCPUCompareXML(caps->host.arch, caps->host.cpu, + xmlDesc, failIncompatible); } @@ -20412,17 +20376,16 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, const char *virttype_str, unsigned int flags) { - char *ret = NULL; virQEMUDriverPtr driver = conn->privateData; g_autoptr(virQEMUCaps) qemuCaps = NULL; virArch arch; virDomainVirtType virttype; g_autoptr(virDomainCaps) domCaps = NULL; - virCheckFlags(0, ret); + virCheckFlags(0, NULL); if (virConnectGetDomainCapabilitiesEnsureACL(conn) < 0) - return ret; + return NULL; qemuCaps = virQEMUCapsCacheLookupDefault(driver->qemuCapsCache, emulatorbin, @@ -20431,16 +20394,14 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, machine, &arch, &virttype, &machine); if (!qemuCaps) - goto cleanup; + return NULL; if (!(domCaps = virQEMUDriverGetDomainCapabilities(driver, qemuCaps, machine, arch, virttype))) - goto cleanup; + return NULL; - ret = virDomainCapsFormat(domCaps); - cleanup: - return ret; + return virDomainCapsFormat(domCaps); } @@ -21107,7 +21068,6 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, const char *frontendalias; const char *backendalias; const char *backendstoragealias; - int ret = -1; /* * This helps to keep logs clean from error messages on getting stats @@ -21134,7 +21094,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, /* alias may be NULL if the VM is not running */ if (disk->info.alias && !(alias = qemuDomainStorageAlias(disk->info.alias, n->id))) - goto cleanup; + return -1; qemuDomainGetStatsOneBlockRefreshNamed(n, alias, stats, nodestats); @@ -21144,24 +21104,24 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, } if (qemuDomainGetStatsBlockExportHeader(disk, n, *recordnr, params) < 0) - goto cleanup; + return -1; /* The following stats make sense only for the frontend device */ if (n == disk->src) { if (qemuDomainGetStatsBlockExportFrontend(frontendalias, stats, *recordnr, params) < 0) - goto cleanup; + return -1; } if (qemuDomainGetStatsOneBlock(driver, cfg, dom, params, backendalias, n, *recordnr, stats) < 0) - goto cleanup; + return -1; if (qemuDomainGetStatsBlockExportBackendStorage(backendstoragealias, stats, *recordnr, params) < 0) - goto cleanup; + return -1; (*recordnr)++; @@ -21169,10 +21129,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, break; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -22534,31 +22491,26 @@ qemuNodeGetSEVInfo(virConnectPtr conn, unsigned int flags) { virQEMUDriverPtr driver = conn->privateData; - int ret = -1; g_autoptr(virQEMUCaps) qemucaps = NULL; if (virNodeGetSevInfoEnsureACL(conn) < 0) - return ret; + return -1; qemucaps = virQEMUCapsCacheLookupByArch(driver->qemuCapsCache, virArchFromHost()); if (!qemucaps) - goto cleanup; + return -1; if (!virQEMUCapsGet(qemucaps, QEMU_CAPS_SEV_GUEST)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("QEMU does not support SEV guest")); - goto cleanup; + return -1; } if (qemuGetSEVInfoToParams(qemucaps, params, nparams, flags) < 0) - goto cleanup; - - ret = 0; - - cleanup: + return -1; - return ret; + return 0; } -- 2.21.0

This patch changes all virAsprintf calls to use the GLib API g_strdup_printf in qemu_driver.c Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a263393626..c9b3ed877f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -402,7 +402,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, priv = vm->privateData; - if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) < 0) { + if (!(snapDir = g_strdup_printf("%s/%s", baseDir, vm->def->name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to allocate memory for " "snapshot directory for domain %s"), @@ -427,7 +427,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, kill the whole process */ VIR_INFO("Loading snapshot file '%s'", entry->d_name); - if (virAsprintf(&fullpath, "%s/%s", snapDir, entry->d_name) < 0) { + if (!(fullpath = g_strdup_printf("%s/%s", snapDir, entry->d_name))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to allocate memory for path")); continue; @@ -514,7 +514,7 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, virObjectLock(vm); priv = vm->privateData; - if (virAsprintf(&chkDir, "%s/%s", baseDir, vm->def->name) < 0) { + if (!(chkDir = g_strdup_printf("%s/%s", baseDir, vm->def->name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to allocate memory for " "checkpoint directory for domain %s"), @@ -539,7 +539,7 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm, kill the whole process */ VIR_INFO("Loading checkpoint file '%s'", entry->d_name); - if (virAsprintf(&fullpath, "%s/%s", chkDir, entry->d_name) < 0) { + if (!(fullpath = g_strdup_printf("%s/%s", chkDir, entry->d_name))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to allocate memory for path")); continue; @@ -690,7 +690,7 @@ qemuStateInitialize(bool privileged, if (!(qemu_driver->config = cfg = virQEMUDriverConfigNew(privileged))) goto error; - if (virAsprintf(&driverConf, "%s/qemu.conf", cfg->configBaseDir) < 0) + if (!(driverConf = g_strdup_printf("%s/qemu.conf", cfg->configBaseDir))) goto error; if (virQEMUDriverConfigLoadFile(cfg, driverConf, privileged) < 0) @@ -1359,10 +1359,10 @@ qemuGetSchedInfo(unsigned long long *cpuWait, /* In general, we cannot assume pid_t fits in int; but /proc parsing * is specific to Linux where int works fine. */ if (tid) - ret = virAsprintf(&proc, "/proc/%d/task/%d/sched", (int)pid, (int)tid); + proc = g_strdup_printf("/proc/%d/task/%d/sched", (int)pid, (int)tid); else - ret = virAsprintf(&proc, "/proc/%d/sched", (int)pid); - if (ret < 0) + proc = g_strdup_printf("/proc/%d/sched", (int)pid); + if (!proc) goto cleanup; ret = -1; @@ -1426,15 +1426,14 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, unsigned long long usertime = 0, systime = 0; long rss = 0; int cpu = 0; - int ret; /* In general, we cannot assume pid_t fits in int; but /proc parsing * is specific to Linux where int works fine. */ if (tid) - ret = virAsprintf(&proc, "/proc/%d/task/%d/stat", (int)pid, tid); + proc = g_strdup_printf("/proc/%d/task/%d/stat", (int)pid, tid); else - ret = virAsprintf(&proc, "/proc/%d/stat", (int)pid); - if (ret < 0) + proc = g_strdup_printf("/proc/%d/stat", (int)pid); + if (!proc) return -1; pidinfo = fopen(proc, "r"); @@ -3521,7 +3520,7 @@ qemuDomainManagedSavePath(virQEMUDriverPtr driver, virDomainObjPtr vm) char *ret; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - if (virAsprintf(&ret, "%s/%s.save", cfg->saveDir, vm->def->name) < 0) + if (!(ret = g_strdup_printf("%s/%s.save", cfg->saveDir, vm->def->name))) return NULL; return ret; @@ -4032,7 +4031,7 @@ qemuDomainScreenshot(virDomainPtr dom, } } - if (virAsprintf(&tmp, "%s/qemu.screendump.XXXXXX", cfg->cacheDir) < 0) + if (!(tmp = g_strdup_printf("%s/qemu.screendump.XXXXXX", cfg->cacheDir))) goto endjob; if ((tmp_fd = mkostemp(tmp, O_CLOEXEC)) == -1) { @@ -4095,10 +4094,7 @@ getAutoDumpPath(virQEMUDriverPtr driver, localtime_r(&curtime, &time_info); strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info); - ignore_value(virAsprintf(&dumpfile, "%s/%s-%s", - cfg->autoDumpPath, - domname, - timestr)); + dumpfile = g_strdup_printf("%s/%s-%s", cfg->autoDumpPath, domname, timestr); return dumpfile; } @@ -5883,7 +5879,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, virDomainIOThreadIDDefPtr iothrid; virJSONValuePtr props = NULL; - if (virAsprintf(&alias, "iothread%u", iothread_id) < 0) + if (!(alias = g_strdup_printf("iothread%u", iothread_id))) return -1; if (qemuMonitorCreateObjectProps(&props, "iothread", alias, NULL) < 0) @@ -6001,7 +5997,7 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver, int new_niothreads = 0; qemuMonitorIOThreadInfoPtr *new_iothreads = NULL; - if (virAsprintf(&alias, "iothread%u", iothread_id) < 0) + if (!(alias = g_strdup_printf("iothread%u", iothread_id))) return -1; qemuDomainObjEnterMonitor(driver, vm); @@ -11986,7 +11982,7 @@ qemuDomainMemoryPeek(virDomainPtr dom, if (virDomainObjCheckActive(vm) < 0) goto endjob; - if (virAsprintf(&tmp, "%s/qemu.mem.XXXXXX", cfg->cacheDir) < 0) + if (!(tmp = g_strdup_printf("%s/qemu.mem.XXXXXX", cfg->cacheDir))) goto endjob; /* Create a temporary filename. */ -- 2.21.0

On 10/16/19 4:54 PM, Daniel Henrique Barboza wrote:
This patch changes all virAsprintf calls to use the GLib API g_strdup_printf in qemu_driver.c
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a263393626..c9b3ed877f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -402,7 +402,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
priv = vm->privateData;
- if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) < 0) { + if (!(snapDir = g_strdup_printf("%s/%s", baseDir, vm->def->name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to allocate memory for " "snapshot directory for domain %s"), @@ -427,7 +427,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, kill the whole process */ VIR_INFO("Loading snapshot file '%s'", entry->d_name);
No objection to this patch specifically, but if g_strdup_printf is a drop in replacement for virAsprintf, I think conversion should be done in a mass change-the-world series like Jano has done for other pieces. - Cole

On 10/17/19 10:04 AM, Cole Robinson wrote:
On 10/16/19 4:54 PM, Daniel Henrique Barboza wrote:
This patch changes all virAsprintf calls to use the GLib API g_strdup_printf in qemu_driver.c
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a263393626..c9b3ed877f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -402,7 +402,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
priv = vm->privateData;
- if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) < 0) { + if (!(snapDir = g_strdup_printf("%s/%s", baseDir, vm->def->name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to allocate memory for " "snapshot directory for domain %s"), @@ -427,7 +427,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, kill the whole process */ VIR_INFO("Loading snapshot file '%s'", entry->d_name);
No objection to this patch specifically, but if g_strdup_printf is a drop in replacement for virAsprintf, I think conversion should be done in a mass change-the-world series like Jano has done for other pieces.
Fair enough. Sadly I've just sent another series in which I did the same thing for qemu_process.c :| DHB
- Cole

On Thu, Oct 17, 2019 at 09:04:05 -0400, Cole Robinson wrote:
On 10/16/19 4:54 PM, Daniel Henrique Barboza wrote:
This patch changes all virAsprintf calls to use the GLib API g_strdup_printf in qemu_driver.c
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a263393626..c9b3ed877f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -402,7 +402,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
priv = vm->privateData;
- if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) < 0) { + if (!(snapDir = g_strdup_printf("%s/%s", baseDir, vm->def->name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to allocate memory for " "snapshot directory for domain %s"), @@ -427,7 +427,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, kill the whole process */ VIR_INFO("Loading snapshot file '%s'", entry->d_name);
No objection to this patch specifically, but if g_strdup_printf is a drop in replacement for virAsprintf, I think conversion should be done in a mass change-the-world series like Jano has done for other pieces.
It actually is not a drop in replacement. virAsprintf is a wrapper around g_vasprintf and aborts on OOM. On the other hand g_strdup_printf returns NULL and doesn't set any error. I think our documentation is wrong in suggesting we should use g_strdup_printf directly and I believe this patch should be reverted. Jirka

On Thu, Oct 17, 2019 at 03:30:26PM +0200, Jiri Denemark wrote:
On Thu, Oct 17, 2019 at 09:04:05 -0400, Cole Robinson wrote:
On 10/16/19 4:54 PM, Daniel Henrique Barboza wrote:
This patch changes all virAsprintf calls to use the GLib API g_strdup_printf in qemu_driver.c
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a263393626..c9b3ed877f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -402,7 +402,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
priv = vm->privateData;
- if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) < 0) { + if (!(snapDir = g_strdup_printf("%s/%s", baseDir, vm->def->name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to allocate memory for " "snapshot directory for domain %s"), @@ -427,7 +427,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, kill the whole process */ VIR_INFO("Loading snapshot file '%s'", entry->d_name);
No objection to this patch specifically, but if g_strdup_printf is a drop in replacement for virAsprintf, I think conversion should be done in a mass change-the-world series like Jano has done for other pieces.
It actually is not a drop in replacement. virAsprintf is a wrapper around g_vasprintf and aborts on OOM. On the other hand g_strdup_printf returns NULL and doesn't set any error. I think our documentation is wrong in suggesting we should use g_strdup_printf directly and I believe this patch should be reverted.
g_strdup_printf was supposed to abort on OOM, but due to bug it does not do so on Linux at least. On Windows it will. This is fixed in git master glib. I think it is nice to use g_strdup_printf directly, so we should create transparent fix in the same way gnulib fixes things like this: #if !GLIB_CHECK_VERSION(2, 64, 0) static inline char *vir_g_strdup_printf(const char *msg, ...) { va_list args; char *ret; va_start(msg, args); ret = g_stdup_vprintf(msg, args); if (!ret) abort(); va_end(args); return ret } static inline char *vir_g_strdup_vprintf(const char *msg, va_list args) { char *ret; ret = g_stdup_vprintf(msg, args); if (!ret) abort(); return ret } # define g_strdup_vprintf vir_g_strdup_vprintf #endif (untested code) We can then just drop this compat when we set min glib to 2.64 in future and not have to update all the callers. Regards. Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 10/17/19 10:51 AM, Daniel P. Berrangé wrote:
On Thu, Oct 17, 2019 at 03:30:26PM +0200, Jiri Denemark wrote:
On 10/16/19 4:54 PM, Daniel Henrique Barboza wrote:
This patch changes all virAsprintf calls to use the GLib API g_strdup_printf in qemu_driver.c
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a263393626..c9b3ed877f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -402,7 +402,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
priv = vm->privateData;
- if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) < 0) { + if (!(snapDir = g_strdup_printf("%s/%s", baseDir, vm->def->name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to allocate memory for " "snapshot directory for domain %s"), @@ -427,7 +427,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, kill the whole process */ VIR_INFO("Loading snapshot file '%s'", entry->d_name);
No objection to this patch specifically, but if g_strdup_printf is a drop in replacement for virAsprintf, I think conversion should be done in a mass change-the-world series like Jano has done for other pieces. It actually is not a drop in replacement. virAsprintf is a wrapper around g_vasprintf and aborts on OOM. On the other hand g_strdup_printf returns NULL and doesn't set any error. I think our documentation is wrong in suggesting we should use g_strdup_printf directly and I believe
On Thu, Oct 17, 2019 at 09:04:05 -0400, Cole Robinson wrote: this patch should be reverted. g_strdup_printf was supposed to abort on OOM, but due to bug it does not do so on Linux at least. On Windows it will.
This is fixed in git master glib.
I think it is nice to use g_strdup_printf directly, so we should create transparent fix in the same way gnulib fixes things like this:
#if !GLIB_CHECK_VERSION(2, 64, 0) static inline char *vir_g_strdup_printf(const char *msg, ...) { va_list args; char *ret; va_start(msg, args); ret = g_stdup_vprintf(msg, args); if (!ret) abort(); va_end(args); return ret } static inline char *vir_g_strdup_vprintf(const char *msg, va_list args) { char *ret; ret = g_stdup_vprintf(msg, args); if (!ret) abort(); return ret } # define g_strdup_vprintf vir_g_strdup_vprintf #endif
(untested code)
We can then just drop this compat when we set min glib to 2.64 in future and not have to update all the callers.
Guess I can play around with changing all the virAsprintf callers to g_strdup_printf then. I'll assume that this wrapper will be already in place when the patches hit upstream. In fact, given that there are g_strdup_printf calls floating around in qemu_driver.c already, I believe this vir_g_strdup_printf proposed here must be pushed as soon as possible. Thanks, DHB
Regards. Daniel

On 10/16/19 10:54 PM, Daniel Henrique Barboza wrote:
changes from v3: - only use the new Glib macros in all patches - different patch split, as suggested by Jano in v3 review - patch 5 (new): g_strdup_printf conversion
changes from v2: - rebased with newer master (67e72053c1) - added an extra patch to convert the existing VIR_AUTO* macros to g_auto* ones, all at once, to avoid the case where a method will have both VIR_AUTO* and g_auto* macros at the same time.
changes from v1: - addressed review concerns made by Erik - added more cleanups, as suggested by Erik
v3: https://www.redhat.com/archives/libvir-list/2019-October/msg00918.html v2: https://www.redhat.com/archives/libvir-list/2019-September/msg01452.html v1: https://www.redhat.com/archives/libvir-list/2019-September/msg00719.html
Daniel Henrique Barboza (5): qemu_driver: use g_auto* in some functions qemu_driver: use g_autoptr() when possible qemu_driver: use g_autofree when possible qemu_driver: remove unused 'cleanup' labels after g_auto*() changes qemu_driver: use g_strdup_printf
src/qemu/qemu_driver.c | 721 ++++++++++++++--------------------------- 1 file changed, 242 insertions(+), 479 deletions(-)
I'll replace qemu_driver with qemu_driver.c in subjects because you're touching qemu_driver.c only. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> And pushed. Michal
participants (5)
-
Cole Robinson
-
Daniel Henrique Barboza
-
Daniel P. Berrangé
-
Jiri Denemark
-
Michal Privoznik