On 9/9/19 1:00 PM, Michal Privoznik wrote:

Our naming rules prefer qemuObjectOperation() scheme rather than qemuOperationObject() for function names. These were not honoured in recent commits to qemu_conf.c.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

src/qemu/qemu_conf.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 86b2eef060..32d411e536 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1679,7 +1679,7 @@ qemuSharedDeviceEntryRemove(virQEMUDriverPtr driver,

static int -qemuAddRemoveSharedDiskInternal(virQEMUDriverPtr driver, +qemuSharedDiskAddRemoveInternal(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, const char *name, bool addDisk) @@ -1730,7 +1730,7 @@ qemuAddSharedDisk(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, const char *name) { - return qemuAddRemoveSharedDiskInternal(driver, disk, name, true); + return qemuSharedDiskAddRemoveInternal(driver, disk, name, true); }

@@ -1768,7 +1768,7 @@ qemuGetHostdevPath(virDomainHostdevDefPtr hostdev)

static int -qemuAddRemoveSharedHostdevInternal(virQEMUDriverPtr driver, +qemuSharedHostdevAddRemoveInternal(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev, const char *name, bool addDevice) @@ -1803,7 +1803,7 @@ qemuAddRemoveSharedHostdevInternal(virQEMUDriverPtr driver, }

static int -qemuAddRemoveSharedDeviceInternal(virQEMUDriverPtr driver, +qemuSharedDeviceAddRemoveInternal(virQEMUDriverPtr driver, virDomainDeviceDefPtr dev, const char *name, bool addDevice) @@ -1813,10 +1813,10 @@ qemuAddRemoveSharedDeviceInternal(virQEMUDriverPtr driver, * which is only valid for block disk and scsi host device. */ if (dev->type == VIR_DOMAIN_DEVICE_DISK) - return qemuAddRemoveSharedDiskInternal(driver, dev->data.disk, + return qemuSharedDiskAddRemoveInternal(driver, dev->data.disk, name, addDevice); else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) - return qemuAddRemoveSharedHostdevInternal(driver, dev->data.hostdev, + return qemuSharedHostdevAddRemoveInternal(driver, dev->data.hostdev, name, addDevice); else return 0; @@ -1837,7 +1837,7 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, virDomainDeviceDefPtr dev, const char *name) { - return qemuAddRemoveSharedDeviceInternal(driver, dev, name, true); + return qemuSharedDeviceAddRemoveInternal(driver, dev, name, true); }

@@ -1846,7 +1846,7 @@ qemuRemoveSharedDisk(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, const char *name) { - return qemuAddRemoveSharedDiskInternal(driver, disk, name, false); + return qemuSharedDiskAddRemoveInternal(driver, disk, name, false); }

@@ -1864,7 +1864,7 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, virDomainDeviceDefPtr dev, const char *name) { - return qemuAddRemoveSharedDeviceInternal(driver, dev, name, false); + return qemuSharedDeviceAddRemoveInternal(driver, dev, name, false); }

On Mon, Sep 09, 2019 at 06:00:23PM +0200, Michal Privoznik wrote:

There are two 'cleanup' labels - one in virQEMUDriverConfigHugeTLBFSInit() and the other in virQEMUDriverConfigSetDefaults() that do nothing more than return and integer value. No memory freeing or anything important is done there. Drop them in favour of returning immediately.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_conf.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 32d411e536..f11df03cf8 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -402,18 +402,12 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs, const char *path, bool deflt) { - int ret = -1; - - if (VIR_STRDUP(hugetlbfs->mnt_dir, path) < 0) - goto cleanup; - - if (virFileGetHugepageSize(path, &hugetlbfs->size) < 0) - goto cleanup; + if (VIR_STRDUP(hugetlbfs->mnt_dir, path) < 0 || + virFileGetHugepageSize(path, &hugetlbfs->size) < 0) + return -1;

I prefer the old code-style to have two separate if conditions but that's just my personal view. We don't have a syntax-check rule for that but the body should be wrapped in curly braces [1]. I know that our code doesn't follow that rule but we should avoid introducing new occurrences.

hugetlbfs->deflt = deflt; - ret = 0; - cleanup: - return ret; + return 0; }

@@ -1172,8 +1166,6 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg) int virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) { - int ret = -1; - #define SET_TLS_SECRET_UUID_DEFAULT(val) \ do { \ if (!cfg->val## TLSx509certdir && \ @@ -1181,7 +1173,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) cfg->defaultTLSx509secretUUID) { \ if (VIR_STRDUP(cfg->val## TLSx509secretUUID, \ cfg->defaultTLSx509secretUUID) < 0) \ - goto cleanup; \ + return -1; \ } \ } while (0)

@@ -1205,11 +1197,11 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) if (virFileExists(SYSCONFDIR "/pki/libvirt-"#val)) { \ if (VIR_STRDUP(cfg->val ## TLSx509certdir, \ SYSCONFDIR "/pki/libvirt-"#val) < 0) \ - goto cleanup; \ + return -1; \ } else { \ if (VIR_STRDUP(cfg->val ## TLSx509certdir, \ cfg->defaultTLSx509certdir) < 0) \ - goto cleanup; \ + return -1; \

I would fix all the cases above to use curly-braces but it's a existing code so I'll leave that up to you. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

On Mon, Sep 09, 2019 at 06:00:24PM +0200, Michal Privoznik wrote:

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_conf.c | 130 +++++++++++++------------------------------ 1 file changed, 40 insertions(+), 90 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f11df03cf8..c3255a6f54 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -181,37 +181,26 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) virGetGroupID("tss", &cfg->swtpm_group) < 0) cfg->swtpm_group = 0; /* fall back to root */ } else { - char *rundir; - char *cachedir; + VIR_AUTOFREE(char *) rundir = NULL; + VIR_AUTOFREE(char *) cachedir = NULL;

cachedir = virGetUserCacheDirectory(); if (!cachedir) goto error;

- if (virAsprintf(&cfg->logDir, - "%s/qemu/log", cachedir) < 0) { - VIR_FREE(cachedir); + if (virAsprintf(&cfg->logDir, "%s/qemu/log", cachedir) < 0) goto error; - } - if (virAsprintf(&cfg->swtpmLogDir, - "%s/qemu/log", cachedir) < 0) { - VIR_FREE(cachedir); + if (virAsprintf(&cfg->swtpmLogDir, "%s/qemu/log", cachedir) < 0) goto error; - } - if (virAsprintf(&cfg->cacheDir, "%s/qemu/cache", cachedir) < 0) { - VIR_FREE(cachedir); + if (virAsprintf(&cfg->cacheDir, "%s/qemu/cache", cachedir) < 0) goto error; - } - VIR_FREE(cachedir);

rundir = virGetUserRuntimeDirectory(); if (!rundir) goto error; if (virAsprintf(&cfg->stateDir, "%s/qemu/run", rundir) < 0) { - VIR_FREE(rundir); goto error; }

This fails syntax-check, please fix before pushing.

- VIR_FREE(rundir);

if (virAsprintf(&cfg->swtpmStateDir, "%s/swtpm", cfg->stateDir) < 0) goto error; @@ -1261,7 +1250,7 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) { size_t i, j; virCapsPtr caps; - virSecurityManagerPtr *sec_managers = NULL; + VIR_AUTOFREE(virSecurityManagerPtr) *sec_managers = NULL; /* Security driver data */ const char *doi, *model, *lbl, *type; const int virtTypes[] = {VIR_DOMAIN_VIRT_KVM, @@ -1308,12 +1297,10 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) VIR_DEBUG("Initialized caps for security driver \"%s\" with " "DOI \"%s\"", model, doi); } - VIR_FREE(sec_managers);

return caps;

error: - VIR_FREE(sec_managers); virObjectUnref(caps); return NULL; } @@ -1485,31 +1472,26 @@ qemuCheckUnprivSGIO(virHashTablePtr sharedDevices, const char *device_path, int sgio) { - char *sysfs_path = NULL; - char *key = NULL; + VIR_AUTOFREE(char *) sysfs_path = NULL; + VIR_AUTOFREE(char *) key = NULL; int val; - int ret = -1;

if (!(sysfs_path = virGetUnprivSGIOSysfsPath(device_path, NULL))) - goto cleanup; + return -1;

/* It can't be conflict if unpriv_sgio is not supported by kernel. */ - if (!virFileExists(sysfs_path)) { - ret = 0; - goto cleanup; - } + if (!virFileExists(sysfs_path)) + return 0;

if (!(key = qemuGetSharedDeviceKey(device_path))) - goto cleanup; + return -1;

/* It can't be conflict if no other domain is sharing it. */ - if (!(virHashLookup(sharedDevices, key))) { - ret = 0; - goto cleanup; - } + if (!(virHashLookup(sharedDevices, key))) + return 0;

if (virGetDeviceUnprivSGIO(device_path, NULL, &val) < 0) - goto cleanup; + return -1;

/* Error message on failure needs to be handled in caller * since there is more specific knowledge of device @@ -1519,16 +1501,10 @@ qemuCheckUnprivSGIO(virHashTablePtr sharedDevices, sgio == VIR_DOMAIN_DEVICE_SGIO_DEFAULT)) || (val == 1 && sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED))) { - ret = -2; - goto cleanup; + return -2; }

- ret = 0; - - cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(key); - return ret; + return 0; }

@@ -1674,7 +1650,7 @@ qemuSharedDiskAddRemoveInternal(virQEMUDriverPtr driver, const char *name, bool addDisk) { - char *key = NULL; + VIR_AUTOFREE(char *) key = NULL; int ret = -1;

if (virStorageSourceIsEmpty(disk->src) || @@ -1701,7 +1677,6 @@ qemuSharedDiskAddRemoveInternal(virQEMUDriverPtr driver, ret = 0; cleanup: qemuDriverUnlock(driver); - VIR_FREE(key); return ret; }

@@ -1739,7 +1714,7 @@ qemuGetHostdevPath(virDomainHostdevDefPtr hostdev) { virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; - char *dev_name = NULL; + VIR_AUTOFREE(char *) dev_name = NULL; char *dev_path = NULL;

if (!(dev_name = virSCSIDeviceGetDevName(NULL, @@ -1747,12 +1722,9 @@ qemuGetHostdevPath(virDomainHostdevDefPtr hostdev) scsihostsrc->bus, scsihostsrc->target, scsihostsrc->unit))) - goto cleanup; + return NULL;

ignore_value(virAsprintf(&dev_path, "/dev/%s", dev_name)); - - cleanup: - VIR_FREE(dev_name); return dev_path; }

@@ -1763,18 +1735,16 @@ qemuSharedHostdevAddRemoveInternal(virQEMUDriverPtr driver, const char *name, bool addDevice) { - char *dev_path = NULL; - char *key = NULL; + VIR_AUTOFREE(char *) dev_path = NULL; + VIR_AUTOFREE(char *) key = NULL; int ret = -1;

if (!qemuIsSharedHostdev(hostdev)) return 0;

- if (!(dev_path = qemuGetHostdevPath(hostdev))) - goto cleanup; - - if (!(key = qemuGetSharedDeviceKey(dev_path))) - goto cleanup; + if (!(dev_path = qemuGetHostdevPath(hostdev)) || + !(key = qemuGetSharedDeviceKey(dev_path))) + return -1;

Same comment as in the other patch, I prefer two separate conditions, but if you decide to keep it like this add a curly braces.

qemuDriverLock(driver);

@@ -1785,11 +1755,7 @@ qemuSharedHostdevAddRemoveInternal(virQEMUDriverPtr driver,

qemuDriverUnlock(driver);

- cleanup: - VIR_FREE(dev_path); - VIR_FREE(key); return ret; - }

static int @@ -1863,10 +1829,9 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk = NULL; virDomainHostdevDefPtr hostdev = NULL; - char *sysfs_path = NULL; + VIR_AUTOFREE(char *) sysfs_path = NULL; const char *path = NULL; int val = -1; - int ret = -1;

/* "sgio" is only valid for block disk; cdrom * and floopy disk can have empty source. @@ -1889,7 +1854,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'sgio' is not supported for SCSI " "generic device yet ")); - goto cleanup; + return -1; }

return 0; @@ -1898,7 +1863,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) }

if (!(sysfs_path = virGetUnprivSGIOSysfsPath(path, NULL))) - goto cleanup; + return -1;

/* By default, filter the SG_IO commands, i.e. set unpriv_sgio to 0. */ val = (disk->sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED); @@ -1909,13 +1874,9 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) */ if ((virFileExists(sysfs_path) || val == 1) && virSetDeviceUnprivSGIO(path, NULL, val) < 0) - goto cleanup; + return -1;

- ret = 0; - - cleanup: - VIR_FREE(sysfs_path); - return ret; + return 0; }

int qemuDriverAllocateID(virQEMUDriverPtr driver) @@ -1951,14 +1912,12 @@ char * qemuGetDomainHugepagePath(const virDomainDef *def, virHugeTLBFSPtr hugepage) { - char *base = qemuGetBaseHugepagePath(hugepage); - char *domPath = virDomainDefGetShortName(def); + VIR_AUTOFREE(char *) base = qemuGetBaseHugepagePath(hugepage); + VIR_AUTOFREE(char *) domPath = virDomainDefGetShortName(def); char *ret = NULL;

if (base && domPath) ignore_value(virAsprintf(&ret, "%s/%s", base, domPath)); - VIR_FREE(domPath); - VIR_FREE(base); return ret; }

@@ -2019,20 +1978,15 @@ qemuGetMemoryBackingDomainPath(const virDomainDef *def, virQEMUDriverConfigPtr cfg, char **path) { - char *shortName = NULL; - char *base = NULL; - int ret = -1; + VIR_AUTOFREE(char *) shortName = NULL; + VIR_AUTOFREE(char *) base = NULL;

if (!(shortName = virDomainDefGetShortName(def)) || qemuGetMemoryBackingBasePath(cfg, &base) < 0 || virAsprintf(path, "%s/%s", base, shortName) < 0) - goto cleanup; + return -1;

Would be nice to fix the braces since you are touching this code. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

On 9/9/19 1:00 PM, Michal Privoznik wrote:

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

src/qemu/qemu_conf.c | 139 ++++++++++++++++++++----------------------- 1 file changed, 63 insertions(+), 76 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index c3255a6f54..f805991872 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -105,7 +105,7 @@ qemuDriverUnlock(virQEMUDriverPtr driver)

virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) { - virQEMUDriverConfigPtr cfg; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;

if (virQEMUConfigInitialize() < 0) return NULL; @@ -117,9 +117,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)

if (privileged) { if (virGetUserID(QEMU_USER, &cfg->user) < 0) - goto error; + return NULL; if (virGetGroupID(QEMU_GROUP, &cfg->group) < 0) - goto error; + return NULL; } else { cfg->user = (uid_t)-1; cfg->group = (gid_t)-1; @@ -132,48 +132,48 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) if (privileged) { if (virAsprintf(&cfg->logDir, "%s/log/libvirt/qemu", LOCALSTATEDIR) < 0) - goto error; + return NULL;

if (virAsprintf(&cfg->swtpmLogDir, "%s/log/swtpm/libvirt/qemu", LOCALSTATEDIR) < 0) - goto error; + return NULL;

if (VIR_STRDUP(cfg->configBaseDir, SYSCONFDIR "/libvirt") < 0) - goto error; + return NULL;

if (virAsprintf(&cfg->stateDir, "%s/libvirt/qemu", RUNSTATEDIR) < 0) - goto error; + return NULL;

if (virAsprintf(&cfg->swtpmStateDir, "%s/libvirt/qemu/swtpm", RUNSTATEDIR) < 0) - goto error; + return NULL;

if (virAsprintf(&cfg->cacheDir, "%s/cache/libvirt/qemu", LOCALSTATEDIR) < 0) - goto error; + return NULL;

if (virAsprintf(&cfg->libDir, "%s/lib/libvirt/qemu", LOCALSTATEDIR) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->saveDir, "%s/save", cfg->libDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->snapshotDir, "%s/snapshot", cfg->libDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->checkpointDir, "%s/checkpoint", cfg->libDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->autoDumpPath, "%s/dump", cfg->libDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->channelTargetDir, "%s/channel/target", cfg->libDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->nvramDir, "%s/nvram", cfg->libDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->memoryBackingDir, "%s/ram", cfg->libDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->swtpmStorageDir, "%s/lib/libvirt/swtpm", LOCALSTATEDIR) < 0) - goto error; + return NULL; if (!virDoesUserExist("tss") || virGetUserID("tss", &cfg->swtpm_user) < 0) cfg->swtpm_user = 0; /* fall back to root */ @@ -186,58 +186,58 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)

cachedir = virGetUserCacheDirectory(); if (!cachedir) - goto error; + return NULL;

if (virAsprintf(&cfg->logDir, "%s/qemu/log", cachedir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->swtpmLogDir, "%s/qemu/log", cachedir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->cacheDir, "%s/qemu/cache", cachedir) < 0) - goto error; + return NULL;

rundir = virGetUserRuntimeDirectory(); if (!rundir) - goto error; + return NULL; if (virAsprintf(&cfg->stateDir, "%s/qemu/run", rundir) < 0) { - goto error; + return NULL; }

if (virAsprintf(&cfg->swtpmStateDir, "%s/swtpm", cfg->stateDir) < 0) - goto error; + return NULL;

if (!(cfg->configBaseDir = virGetUserConfigDirectory())) - goto error; + return NULL;

if (virAsprintf(&cfg->libDir, "%s/qemu/lib", cfg->configBaseDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->saveDir, "%s/qemu/save", cfg->configBaseDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->snapshotDir, "%s/qemu/snapshot", cfg->configBaseDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->checkpointDir, "%s/qemu/checkpoint", cfg->configBaseDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->autoDumpPath, "%s/qemu/dump", cfg->configBaseDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->channelTargetDir, "%s/qemu/channel/target", cfg->configBaseDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->nvramDir, "%s/qemu/nvram", cfg->configBaseDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->memoryBackingDir, "%s/qemu/ram", cfg->configBaseDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->swtpmStorageDir, "%s/qemu/swtpm", cfg->configBaseDir) < 0) - goto error; + return NULL; cfg->swtpm_user = (uid_t)-1; cfg->swtpm_group = (gid_t)-1; }

if (virAsprintf(&cfg->configDir, "%s/qemu", cfg->configBaseDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->autostartDir, "%s/qemu/autostart", cfg->configBaseDir) < 0) - goto error; + return NULL; if (virAsprintf(&cfg->slirpStateDir, "%s/slirp", cfg->stateDir) < 0) - goto error; + return NULL;

/* Set the default directory to find TLS X.509 certificates. * This will then be used as a fallback if the service specific @@ -245,13 +245,13 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) */ if (VIR_STRDUP(cfg->defaultTLSx509certdir, SYSCONFDIR "/pki/qemu") < 0) - goto error; + return NULL;

if (VIR_STRDUP(cfg->vncListen, VIR_LOOPBACK_IPV4_ADDR) < 0) - goto error; + return NULL;

if (VIR_STRDUP(cfg->spiceListen, VIR_LOOPBACK_IPV4_ADDR) < 0) - goto error; + return NULL;

cfg->remotePortMin = QEMU_REMOTE_PORT_MIN; cfg->remotePortMax = QEMU_REMOTE_PORT_MAX; @@ -269,13 +269,13 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) virFileFindHugeTLBFS(&cfg->hugetlbfs, &cfg->nhugetlbfs) < 0) { /* This however is not implemented on all platforms. */ if (virGetLastErrorCode() != VIR_ERR_NO_SUPPORT) - goto error; + return NULL; }

if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0 || VIR_STRDUP(cfg->prHelperName, QEMU_PR_HELPER) < 0 || VIR_STRDUP(cfg->slirpHelperName, QEMU_SLIRP_HELPER) < 0) - goto error; + return NULL;

cfg->clearEmulatorCapabilities = true;

@@ -291,23 +291,19 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) cfg->stdioLogD = true;

if (!(cfg->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST))) - goto error; + return NULL;

if (privileged && qemuDomainNamespaceAvailable(QEMU_DOMAIN_NS_MOUNT) && virBitmapSetBit(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT) < 0) - goto error; + return NULL;

if (virFirmwareParseList(DEFAULT_LOADER_NVRAM, &cfg->firmwares, &cfg->nfirmwares) < 0) - goto error; + return NULL;

- return cfg; - - error: - virObjectUnref(cfg); - return NULL; + VIR_RETURN_PTR(cfg); }

@@ -1249,7 +1245,7 @@ virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver) virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) { size_t i, j; - virCapsPtr caps; + VIR_AUTOUNREF(virCapsPtr) caps = NULL; VIR_AUTOFREE(virSecurityManagerPtr) *sec_managers = NULL; /* Security driver data */ const char *doi, *model, *lbl, *type; @@ -1258,17 +1254,17 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver)

/* Basic host arch / guest machine capabilities */ if (!(caps = virQEMUCapsInit(driver->qemuCapsCache))) - goto error; + return NULL;

if (virGetHostUUID(caps->host.host_uuid)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot get the host uuid")); - goto error; + return NULL; }

/* access sec drivers and create a sec model for each one */ if (!(sec_managers = qemuSecurityGetNested(driver->securityManager))) - goto error; + return NULL;

/* calculate length */ for (i = 0; sec_managers[i]; i++) @@ -1276,7 +1272,7 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) caps->host.nsecModels = i;

if (VIR_ALLOC_N(caps->host.secModels, caps->host.nsecModels) < 0) - goto error; + return NULL;

for (i = 0; sec_managers[i]; i++) { virCapsHostSecModelPtr sm = &caps->host.secModels[i]; @@ -1284,25 +1280,21 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) model = qemuSecurityGetModel(sec_managers[i]); if (VIR_STRDUP(sm->model, model) < 0 || VIR_STRDUP(sm->doi, doi) < 0) - goto error; + return NULL;

for (j = 0; j < ARRAY_CARDINALITY(virtTypes); j++) { lbl = qemuSecurityGetBaseLabel(sec_managers[i], virtTypes[j]); type = virDomainVirtTypeToString(virtTypes[j]); if (lbl && virCapabilitiesHostSecModelAddBaseLabel(sm, type, lbl) < 0) - goto error; + return NULL; }

VIR_DEBUG("Initialized caps for security driver \"%s\" with " "DOI \"%s\"", model, doi); }

- return caps; - - error: - virObjectUnref(caps); - return NULL; + VIR_RETURN_PTR(caps); }

@@ -1389,9 +1381,9 @@ virQEMUDriverGetDomainCapabilities(virQEMUDriverPtr driver, virArch arch, virDomainVirtType virttype) { - virDomainCapsPtr ret = NULL, domCaps = NULL; - virCapsPtr caps = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virDomainCapsPtr) domCaps = NULL; + VIR_AUTOUNREF(virCapsPtr) caps = NULL; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); virHashTablePtr domCapsCache = virQEMUCapsGetDomainCapsCache(qemuCaps); struct virQEMUDriverSearchDomcapsData data = { .path = virQEMUCapsGetBinary(qemuCaps), @@ -1401,7 +1393,7 @@ virQEMUDriverGetDomainCapabilities(virQEMUDriverPtr driver, };

if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; + return NULL;

domCaps = virHashSearch(domCapsCache, virQEMUDriverSearchDomcaps, &data, NULL); @@ -1409,24 +1401,19 @@ virQEMUDriverGetDomainCapabilities(virQEMUDriverPtr driver, /* hash miss, build new domcaps */ if (!(domCaps = virDomainCapsNew(data.path, data.machine, data.arch, data.virttype))) - goto cleanup; + return NULL;

if (virQEMUCapsFillDomainCaps(caps, domCaps, qemuCaps, driver->privileged, cfg->firmwares, cfg->nfirmwares) < 0) - goto cleanup; + return NULL;

if (virHashAddEntry(domCapsCache, machine, domCaps) < 0) - goto cleanup; + return NULL; }

virObjectRef(domCaps); - VIR_STEAL_PTR(ret, domCaps); - cleanup: - virObjectUnref(domCaps); - virObjectUnref(cfg); - virObjectUnref(caps); - return ret; + VIR_RETURN_PTR(domCaps); }

On 9/9/19 1:00 PM, Michal Privoznik wrote:

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

src/bhyve/bhyve_conf.c | 10 +--- src/libvirt-admin.c | 3 +- src/libvirt.c | 4 +- src/libxl/libxl_conf.c | 22 +++----- src/libxl/libxl_driver.c | 8 +-- src/libxl/xen_xl.c | 33 +++++------ src/libxl/xen_xm.c | 19 +++---- src/locking/lock_daemon_config.c | 7 +-- src/locking/lock_driver_lockd.c | 18 +++--- src/locking/lock_driver_sanlock.c | 3 +- src/logging/log_daemon_config.c | 7 +-- src/lxc/lxc_conf.c | 16 ++---- src/lxc/lxc_native.c | 15 ++--- src/qemu/qemu_conf.c | 47 +++++++--------- src/remote/remote_daemon_config.c | 14 ++--- src/security/security_selinux.c | 4 +- src/util/virconf.h | 2 + src/vmx/vmx.c | 3 +- tests/virconftest.c | 93 +++++++++++++------------------ tests/xlconfigtest.c | 8 +-- tests/xmconfigtest.c | 8 +-- tools/virt-login-shell-helper.c | 3 +- 22 files changed, 133 insertions(+), 214 deletions(-)

diff --git a/src/bhyve/bhyve_conf.c b/src/bhyve/bhyve_conf.c index ee9427ea18..946be4a811 100644 --- a/src/bhyve/bhyve_conf.c +++ b/src/bhyve/bhyve_conf.c @@ -70,8 +70,7 @@ int virBhyveLoadDriverConfig(virBhyveDriverConfigPtr cfg, const char *filename) { - virConfPtr conf; - int ret = -1; + VIR_AUTOPTR(virConf) conf = NULL;

if (access(filename, R_OK) == -1) { VIR_INFO("Could not read bhyve config file %s", filename); @@ -83,12 +82,9 @@ virBhyveLoadDriverConfig(virBhyveDriverConfigPtr cfg,

if (virConfGetValueString(conf, "firmware_dir", &cfg->firmwareDir) < 0) - goto cleanup; + return -1;

- ret = 0; - cleanup: - virConfFree(conf); - return ret; + return 0; }

virBhyveDriverConfigPtr diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index ba88f09824..9d5c5b1b7b 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -223,7 +223,7 @@ virAdmConnectOpen(const char *name, unsigned int flags) char *sock_path = NULL; char *alias = NULL; virAdmConnectPtr conn = NULL; - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL; char *uristr = NULL;

if (virAdmInitialize() < 0) @@ -272,7 +272,6 @@ virAdmConnectOpen(const char *name, unsigned int flags) cleanup: VIR_FREE(sock_path); VIR_FREE(uristr); - virConfFree(conf); return conn;

error: diff --git a/src/libvirt.c b/src/libvirt.c index 9650aaa453..19bc05638f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -859,7 +859,7 @@ virConnectOpenInternal(const char *name, size_t i; int res; virConnectPtr ret; - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL; char *uristr = NULL;

ret = virGetConnect(); @@ -1069,14 +1069,12 @@ virConnectOpenInternal(const char *name, goto failed; }

- virConfFree(conf); VIR_FREE(uristr);

return ret;

failed: VIR_FREE(uristr); - virConfFree(conf); virObjectUnref(ret);

return NULL; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 766a726ebc..c76704a11d 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1864,8 +1864,7 @@ libxlDriverConfigGet(libxlDriverPrivatePtr driver) int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, const char *filename) { - virConfPtr conf = NULL; - int ret = -1; + VIR_AUTOPTR(virConf) conf = NULL;

/* defaults for keepalive messages */ cfg->keepAliveInterval = 5; @@ -1880,30 +1879,25 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, }

if (!(conf = virConfReadFile(filename, 0))) - goto cleanup; + return -1;

/* setup autoballoon */ if (libxlGetAutoballoonConf(cfg, conf) < 0) - goto cleanup; + return -1;

if (virConfGetValueString(conf, "lock_manager", &cfg->lockManagerName) < 0) - goto cleanup; + return -1;

if (virConfGetValueInt(conf, "keepalive_interval", &cfg->keepAliveInterval) < 0) - goto cleanup; + return -1;

if (virConfGetValueUInt(conf, "keepalive_count", &cfg->keepAliveCount) < 0) - goto cleanup; + return -1;

if (virConfGetValueBool(conf, "nested_hvm", &cfg->nested_hvm) < 0) - goto cleanup; - - ret = 0; - - cleanup: - virConfFree(conf); - return ret; + return -1;

+ return 0; }

/* diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index fccc1f42e8..45de6b24c6 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2675,7 +2675,7 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, libxlDriverPrivatePtr driver = conn->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainDefPtr def = NULL; - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL; char *xml = NULL;

virCheckFlags(0, NULL); @@ -2712,8 +2712,6 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn,

cleanup: virDomainDefFree(def); - if (conf) - virConfFree(conf); virObjectUnref(cfg); return xml; } @@ -2727,7 +2725,7 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char * nativeFormat, libxlDriverPrivatePtr driver = conn->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainDefPtr def = NULL; - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL; int len = MAX_CONFIG_SIZE; char *ret = NULL;

@@ -2764,8 +2762,6 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char * nativeFormat,

cleanup: virDomainDefFree(def); - if (conf) - virConfFree(conf); virObjectUnref(cfg); return ret; } diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index ca094d30c2..3a41a4ad00 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -2198,52 +2198,47 @@ xenFormatXLDomainChannels(virConfPtr conf, virDomainDefPtr def) virConfPtr xenFormatXL(virDomainDefPtr def, virConnectPtr conn) { - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL;

if (!(conf = virConfNew())) - goto cleanup; + return NULL;

if (xenFormatConfigCommon(conf, def, conn, XEN_CONFIG_FORMAT_XL) < 0) - goto cleanup; + return NULL;

if (xenFormatXLOS(conf, def) < 0) - goto cleanup; + return NULL;

if (xenFormatXLCPUID(conf, def) < 0) - goto cleanup; + return NULL;

#ifdef LIBXL_HAVE_VNUMA if (xenFormatXLDomainVnuma(conf, def) < 0) - goto cleanup; + return NULL; #endif

#ifdef LIBXL_HAVE_BUILDINFO_GRANT_LIMITS if (xenFormatXLGntLimits(conf, def) < 0) - goto cleanup; + return NULL; #endif

if (xenFormatXLDomainDisks(conf, def) < 0) - goto cleanup; + return NULL;

if (xenFormatXLSpice(conf, def) < 0) - goto cleanup; + return NULL;

if (xenFormatXLInputDevs(conf, def) < 0) - goto cleanup; + return NULL;

if (xenFormatXLUSB(conf, def) < 0) - goto cleanup; + return NULL;

if (xenFormatXLUSBController(conf, def) < 0) - goto cleanup; + return NULL;

if (xenFormatXLDomainChannels(conf, def) < 0) - goto cleanup; + return NULL;

- return conf; - - cleanup: - if (conf) - virConfFree(conf); - return NULL; + VIR_RETURN_PTR(conf); } diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c index b8dee0917e..0296e78ddc 100644 --- a/src/libxl/xen_xm.c +++ b/src/libxl/xen_xm.c @@ -599,27 +599,22 @@ virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr def) { - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL;

if (!(conf = virConfNew())) - goto cleanup; + return NULL;

if (xenFormatConfigCommon(conf, def, conn, XEN_CONFIG_FORMAT_XM) < 0) - goto cleanup; + return NULL;

if (xenFormatXMOS(conf, def) < 0) - goto cleanup; + return NULL;

if (xenFormatXMDisks(conf, def) < 0) - goto cleanup; + return NULL;

if (xenFormatXMInputDevs(conf, def) < 0) - goto cleanup; + return NULL;

- return conf; - - cleanup: - if (conf) - virConfFree(conf); - return NULL; + VIR_RETURN_PTR(conf); } diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c index 03feeb02a5..8df7cf89cd 100644 --- a/src/locking/lock_daemon_config.c +++ b/src/locking/lock_daemon_config.c @@ -114,8 +114,7 @@ virLockDaemonConfigLoadFile(virLockDaemonConfigPtr data, const char *filename, bool allow_missing) { - virConfPtr conf; - int ret; + VIR_AUTOPTR(virConf) conf = NULL;

if (allow_missing && access(filename, R_OK) == -1 && @@ -126,7 +125,5 @@ virLockDaemonConfigLoadFile(virLockDaemonConfigPtr data, if (!conf) return -1;

- ret = virLockDaemonConfigLoadOptions(data, conf); - virConfFree(conf); - return ret; + return virLockDaemonConfigLoadOptions(data, conf); } diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 164f83eb4b..67f512a64e 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -81,8 +81,7 @@ static virLockManagerLockDaemonDriverPtr driver;

static int virLockManagerLockDaemonLoadConfig(const char *configFile) { - virConfPtr conf; - int ret = -1; + VIR_AUTOPTR(virConf) conf = NULL;

if (access(configFile, R_OK) == -1) { if (errno != ENOENT) { @@ -98,25 +97,22 @@ static int virLockManagerLockDaemonLoadConfig(const char *configFile) return -1;

if (virConfGetValueBool(conf, "auto_disk_leases", &driver->autoDiskLease) < 0) - goto cleanup; + return -1;

if (virConfGetValueString(conf, "file_lockspace_dir", &driver->fileLockSpaceDir) < 0) - goto cleanup; + return -1;

if (virConfGetValueString(conf, "lvm_lockspace_dir", &driver->lvmLockSpaceDir) < 0) - goto cleanup; + return -1;

if (virConfGetValueString(conf, "scsi_lockspace_dir", &driver->scsiLockSpaceDir) < 0) - goto cleanup; + return -1;

driver->requireLeaseForDisks = !driver->autoDiskLease; if (virConfGetValueBool(conf, "require_lease_for_disks", &driver->requireLeaseForDisks) < 0) - goto cleanup; + return -1;

- ret = 0; - cleanup: - virConfFree(conf); - return ret; + return 0; }

diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index ff0c9be8f7..85a23c7642 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -119,7 +119,7 @@ static int virLockManagerSanlockLoadConfig(virLockManagerSanlockDriverPtr driver, const char *configFile) { - virConfPtr conf; + VIR_AUTOPTR(virConf) conf = NULL; int ret = -1; char *user = NULL; char *group = NULL; @@ -167,7 +167,6 @@ virLockManagerSanlockLoadConfig(virLockManagerSanlockDriverPtr driver,

ret = 0; cleanup: - virConfFree(conf); VIR_FREE(user); VIR_FREE(group); return ret; diff --git a/src/logging/log_daemon_config.c b/src/logging/log_daemon_config.c index ec6d0686f4..c7dfa19b5d 100644 --- a/src/logging/log_daemon_config.c +++ b/src/logging/log_daemon_config.c @@ -120,8 +120,7 @@ virLogDaemonConfigLoadFile(virLogDaemonConfigPtr data, const char *filename, bool allow_missing) { - virConfPtr conf; - int ret; + VIR_AUTOPTR(virConf) conf = NULL;

if (allow_missing && access(filename, R_OK) == -1 && @@ -132,7 +131,5 @@ virLogDaemonConfigLoadFile(virLogDaemonConfigPtr data, if (!conf) return -1;

- ret = virLogDaemonConfigLoadOptions(data, conf); - virConfFree(conf); - return ret; + return virLogDaemonConfigLoadOptions(data, conf); } diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index 2e4cc4f51a..9d1653f437 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -252,8 +252,7 @@ int virLXCLoadDriverConfig(virLXCDriverConfigPtr cfg, const char *filename) { - virConfPtr conf; - int ret = -1; + VIR_AUTOPTR(virConf) conf = NULL;

/* Avoid error from non-existent or unreadable file. */ if (access(filename, R_OK) == -1) @@ -264,21 +263,18 @@ virLXCLoadDriverConfig(virLXCDriverConfigPtr cfg, return -1;

if (virConfGetValueBool(conf, "log_with_libvirtd", &cfg->log_libvirtd) < 0) - goto cleanup; + return -1;

if (virConfGetValueString(conf, "security_driver", &cfg->securityDriverName) < 0) - goto cleanup; + return -1;

if (virConfGetValueBool(conf, "security_default_confined", &cfg->securityDefaultConfined) < 0) - goto cleanup; + return -1;

if (virConfGetValueBool(conf, "security_require_confined", &cfg->securityRequireConfined) < 0) - goto cleanup; + return -1;

- ret = 0; - cleanup: - virConfFree(conf); - return ret; + return 0; }

virLXCDriverConfigPtr virLXCDriverGetConfig(virLXCDriverPtr driver) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index bb8c359147..f9d43a1b3e 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -1079,7 +1079,7 @@ lxcParseConfigString(const char *config, virDomainXMLOptionPtr xmlopt) { virDomainDefPtr vmdef = NULL; - virConfPtr properties = NULL; + VIR_AUTOPTR(virConf) properties = NULL; VIR_AUTOFREE(char *) value = NULL;

if (!(properties = virConfReadString(config, VIR_CONF_FLAG_LXC_FORMAT))) @@ -1192,14 +1192,9 @@ lxcParseConfigString(const char *config, xmlopt, NULL) < 0) goto error;

- goto cleanup; - - error: - virDomainDefFree(vmdef); - vmdef = NULL; - - cleanup: - virConfFree(properties); - return vmdef; + + error: + virDomainDefFree(vmdef); + return NULL; } diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f805991872..dd571cb3aa 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1000,8 +1000,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, const char *filename, bool privileged) { - virConfPtr conf = NULL; - int ret = -1; + VIR_AUTOPTR(virConf) conf = NULL;

/* Just check the file is readable before opening it, otherwise * libvirt emits an error. @@ -1012,67 +1011,63 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, }

if (!(conf = virConfReadFile(filename, 0))) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadDefaultTLSEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadVNCEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadNographicsEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadSPICEEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadSpecificTLSEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadRemoteDisplayEntry(cfg, conf, filename) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadSaveEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadProcessEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadDeviceEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadRPCEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadNetworkEntry(cfg, conf, filename) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadLogEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadNVRAMEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadGlusterDebugEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadSecurityEntry(cfg, conf, privileged) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadMemoryEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadSWTPMEntry(cfg, conf) < 0) - goto cleanup; + return -1;

if (virQEMUDriverConfigLoadCapsFiltersEntry(cfg, conf) < 0) - goto cleanup; + return -1;

- ret = 0; - - cleanup: - virConfFree(conf); - return ret; + return 0; }

diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c index b224e75a18..0eecb20c34 100644 --- a/src/remote/remote_daemon_config.c +++ b/src/remote/remote_daemon_config.c @@ -401,8 +401,7 @@ daemonConfigLoadFile(struct daemonConfig *data, const char *filename, bool allow_missing) { - virConfPtr conf; - int ret; + VIR_AUTOPTR(virConf) conf = NULL;

if (allow_missing && access(filename, R_OK) == -1 && @@ -413,23 +412,18 @@ daemonConfigLoadFile(struct daemonConfig *data, if (!conf) return -1;

- ret = daemonConfigLoadOptions(data, filename, conf); - virConfFree(conf); - return ret; + return daemonConfigLoadOptions(data, filename, conf); }

int daemonConfigLoadData(struct daemonConfig *data, const char *filename, const char *filedata) { - virConfPtr conf; - int ret; + VIR_AUTOPTR(virConf) conf = NULL;

conf = virConfReadString(filedata, 0); if (!conf) return -1;

- ret = daemonConfigLoadOptions(data, filename, conf); - virConfFree(conf); - return ret; + return daemonConfigLoadOptions(data, filename, conf); } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index af7f62dfd9..e879fa39ab 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -635,7 +635,7 @@ virSecuritySELinuxGenNewContext(const char *basecontext, static int virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr) { - virConfPtr selinux_conf; + VIR_AUTOPTR(virConf) selinux_conf = NULL; virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);

data->skipAllLabel = true; @@ -685,7 +685,6 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr) if (!(data->mcs = virHashCreate(10, NULL))) goto error;

- virConfFree(selinux_conf); return 0;

error: @@ -693,7 +692,6 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr) selabel_close(data->label_handle); data->label_handle = NULL; # endif - virConfFree(selinux_conf); VIR_FREE(data->domain_context); VIR_FREE(data->file_context); VIR_FREE(data->content_context); diff --git a/src/util/virconf.h b/src/util/virconf.h index ed9d404a1e..4c0c633aad 100644 --- a/src/util/virconf.h +++ b/src/util/virconf.h @@ -22,6 +22,7 @@

#include "virutil.h" #include "virenum.h" +#include "virautoclean.h"

/** * virConfType: @@ -80,6 +81,7 @@ virConfPtr virConfReadFile(const char *filename, unsigned int flags); virConfPtr virConfReadString(const char *memory, unsigned int flags); int virConfFree(virConfPtr conf); +VIR_DEFINE_AUTOPTR_FUNC(virConf, virConfFree); void virConfFreeValue(virConfValuePtr val); virConfValuePtr virConfGetValue(virConfPtr conf, const char *setting); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 2cc0995783..d8f15b5b47 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1275,7 +1275,7 @@ virVMXParseConfig(virVMXContext *ctx, const char *vmx) { bool success = false; - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL; char *encoding = NULL; char *utf8; virDomainDefPtr def = NULL; @@ -1850,7 +1850,6 @@ virVMXParseConfig(virVMXContext *ctx, def = NULL; }

- virConfFree(conf); VIR_FREE(encoding); VIR_FREE(sched_cpu_affinity); VIR_FREE(sched_cpu_shares); diff --git a/tests/virconftest.c b/tests/virconftest.c index cc2b26687c..f61e4ea9d5 100644 --- a/tests/virconftest.c +++ b/tests/virconftest.c @@ -33,7 +33,7 @@ static int testConfRoundTrip(const void *opaque) { const char *name = opaque; int ret = -1; - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL; int len = 10000; char *buffer = NULL; char *srcfile = NULL; @@ -68,7 +68,6 @@ static int testConfRoundTrip(const void *opaque) VIR_FREE(srcfile); VIR_FREE(dstfile); VIR_FREE(buffer); - virConfFree(conf); return ret; }

@@ -80,7 +79,7 @@ static int testConfMemoryNoNewline(const void *opaque ATTRIBUTE_UNUSED) "string = 'foo'\n" \ "uint = 12345";

- virConfPtr conf = virConfReadString(srcdata, 0); + VIR_AUTOPTR(virConf) conf = virConfReadString(srcdata, 0); int ret = -1; virConfValuePtr val; unsigned long long llvalue; @@ -134,7 +133,6 @@ static int testConfMemoryNoNewline(const void *opaque ATTRIBUTE_UNUSED) ret = 0; cleanup: VIR_FREE(str); - virConfFree(conf); return ret; }

@@ -150,8 +148,7 @@ static int testConfParseInt(const void *opaque ATTRIBUTE_UNUSED) "ssize_t = -87539319\n" \ "string = \"foo\"\n";

- int ret = -1; - virConfPtr conf = virConfReadString(srcdata, 0); + VIR_AUTOPTR(virConf) conf = virConfReadString(srcdata, 0); int iv; unsigned int ui; size_t s; @@ -165,40 +162,40 @@ static int testConfParseInt(const void *opaque ATTRIBUTE_UNUSED) if (virConfGetValueType(conf, "int") != VIR_CONF_LLONG) { fprintf(stderr, "expected a long for 'int'\n"); - goto cleanup; + return -1; }

if (virConfGetValueInt(conf, "int", &iv) < 0) - goto cleanup; + return -1;

if (iv != -1729) { fprintf(stderr, "Expected -1729 got %d\n", iv); - goto cleanup; + return -1; }

if (virConfGetValueInt(conf, "string", &iv) != -1) { fprintf(stderr, "Expected error for 'string' param\n"); - goto cleanup; + return -1; }

if (virConfGetValueType(conf, "uint") != VIR_CONF_ULLONG) { fprintf(stderr, "expected a unsigned long for 'uint'\n"); - goto cleanup; + return -1; }

if (virConfGetValueUInt(conf, "uint", &ui) < 0) - goto cleanup; + return -1;

if (ui != 1729) { fprintf(stderr, "Expected 1729 got %u\n", ui); - goto cleanup; + return -1; }

if (virConfGetValueUInt(conf, "string", &ui) != -1) { fprintf(stderr, "Expected error for 'string' param\n"); - goto cleanup; + return -1; }

@@ -206,20 +203,20 @@ static int testConfParseInt(const void *opaque ATTRIBUTE_UNUSED) if (virConfGetValueType(conf, "llong") != VIR_CONF_LLONG) { fprintf(stderr, "expected a long for 'llong'\n"); - goto cleanup; + return -1; }

if (virConfGetValueLLong(conf, "llong", &l) < 0) - goto cleanup; + return -1;

if (l != -6963472309248) { fprintf(stderr, "Expected -6963472309248 got %lld\n", l); - goto cleanup; + return -1; }

if (virConfGetValueLLong(conf, "string", &l) != -1) { fprintf(stderr, "Expected error for 'string' param\n"); - goto cleanup; + return -1; }

@@ -227,20 +224,20 @@ static int testConfParseInt(const void *opaque ATTRIBUTE_UNUSED) if (virConfGetValueType(conf, "ullong") != VIR_CONF_ULLONG) { fprintf(stderr, "expected a unsigned long for 'ullong'\n"); - goto cleanup; + return -1; }

if (virConfGetValueULLong(conf, "ullong", &ul) < 0) - goto cleanup; + return -1;

if (ul != 6963472309248) { fprintf(stderr, "Expected 6963472309248 got %llu\n", ul); - goto cleanup; + return -1; }

if (virConfGetValueULLong(conf, "string", &ul) != -1) { fprintf(stderr, "Expected error for 'string' param\n"); - goto cleanup; + return -1; }

@@ -248,20 +245,20 @@ static int testConfParseInt(const void *opaque ATTRIBUTE_UNUSED) if (virConfGetValueType(conf, "size_t") != VIR_CONF_ULLONG) { fprintf(stderr, "expected a unsigned long for 'size_T'\n"); - goto cleanup; + return -1; }

if (virConfGetValueSizeT(conf, "size_t", &s) < 0) - goto cleanup; + return -1;

if (s != 87539319) { fprintf(stderr, "Expected 87539319 got %zu\n", s); - goto cleanup; + return -1; }

if (virConfGetValueSizeT(conf, "string", &s) != -1) { fprintf(stderr, "Expected error for 'string' param\n"); - goto cleanup; + return -1; }

@@ -269,26 +266,23 @@ static int testConfParseInt(const void *opaque ATTRIBUTE_UNUSED) if (virConfGetValueType(conf, "ssize_t") != VIR_CONF_LLONG) { fprintf(stderr, "expected a unsigned long for 'ssize_t'\n"); - goto cleanup; + return -1; }

if (virConfGetValueSSizeT(conf, "ssize_t", &ss) < 0) - goto cleanup; + return -1;

if (ss != -87539319) { fprintf(stderr, "Expected -87539319 got %zd\n", ss); - goto cleanup; + return -1; }

if (virConfGetValueSSizeT(conf, "string", &ss) != -1) { fprintf(stderr, "Expected error for 'string' param\n"); - goto cleanup; + return -1; }

- ret = 0; - cleanup: - virConfFree(conf); - return ret; + return 0; }

static int testConfParseBool(const void *opaque ATTRIBUTE_UNUSED) @@ -299,8 +293,7 @@ static int testConfParseBool(const void *opaque ATTRIBUTE_UNUSED) "int = 6963472309248\n" \ "string = \"foo\"\n";

- int ret = -1; - virConfPtr conf = virConfReadString(srcdata, 0); + VIR_AUTOPTR(virConf) conf = virConfReadString(srcdata, 0); bool f = true; bool t = false;

@@ -310,15 +303,15 @@ static int testConfParseBool(const void *opaque ATTRIBUTE_UNUSED) if (virConfGetValueType(conf, "false") != VIR_CONF_ULLONG) { fprintf(stderr, "expected a long for 'false'\n"); - goto cleanup; + return -1; }

if (virConfGetValueBool(conf, "false", &f) < 0) - goto cleanup; + return -1;

if (f != false) { fprintf(stderr, "Expected 0 got %d\n", f); - goto cleanup; + return -1; }

@@ -326,34 +319,30 @@ static int testConfParseBool(const void *opaque ATTRIBUTE_UNUSED) if (virConfGetValueType(conf, "true") != VIR_CONF_ULLONG) { fprintf(stderr, "expected a long for 'true'\n"); - goto cleanup; + return -1; }

if (virConfGetValueBool(conf, "true", &t) < 0) - goto cleanup; + return -1;

if (t != true) { fprintf(stderr, "Expected 1 got %d\n", t); - goto cleanup; + return -1; }

if (virConfGetValueBool(conf, "int", &t) != -1) { fprintf(stderr, "Expected error for 'string' param\n"); - goto cleanup; + return -1; }

if (virConfGetValueBool(conf, "string", &t) != -1) { fprintf(stderr, "Expected error for 'string' param\n"); - goto cleanup; + return -1; }

- - ret = 0; - cleanup: - virConfFree(conf); - return ret; + return 0; }

@@ -364,7 +353,7 @@ static int testConfParseString(const void *opaque ATTRIBUTE_UNUSED) "string = \"foo\"\n";

int ret = -1; - virConfPtr conf = virConfReadString(srcdata, 0); + VIR_AUTOPTR(virConf) conf = virConfReadString(srcdata, 0); char *str = NULL;

if (!conf) @@ -392,7 +381,6 @@ static int testConfParseString(const void *opaque ATTRIBUTE_UNUSED) ret = 0; cleanup: VIR_FREE(str); - virConfFree(conf); return ret; }

@@ -404,7 +392,7 @@ static int testConfParseStringList(const void *opaque ATTRIBUTE_UNUSED) "string = \"foo\"\n";

int ret = -1; - virConfPtr conf = virConfReadString(srcdata, 0); + VIR_AUTOPTR(virConf) conf = virConfReadString(srcdata, 0); char **str = NULL;

if (!conf) @@ -457,7 +445,6 @@ static int testConfParseStringList(const void *opaque ATTRIBUTE_UNUSED) ret = 0; cleanup: virStringListFree(str); - virConfFree(conf); return ret; }

diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index ae0db71293..80ac9b2c9a 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -68,7 +68,7 @@ static int testCompareParseXML(const char *xlcfg, const char *xml, bool replaceVars) { char *gotxlcfgData = NULL; - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL; virConnectPtr conn = NULL; int wrote = 4096; int ret = -1; @@ -113,8 +113,6 @@ testCompareParseXML(const char *xlcfg, const char *xml, bool replaceVars) fail: VIR_FREE(replacedXML); VIR_FREE(gotxlcfgData); - if (conf) - virConfFree(conf); virDomainDefFree(def); virObjectUnref(conn);

@@ -130,7 +128,7 @@ testCompareFormatXML(const char *xlcfg, const char *xml, bool replaceVars) { char *xlcfgData = NULL; char *gotxml = NULL; - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL; int ret = -1; virConnectPtr conn; virDomainDefPtr def = NULL; @@ -165,8 +163,6 @@ testCompareFormatXML(const char *xlcfg, const char *xml, bool replaceVars) ret = 0;

fail: - if (conf) - virConfFree(conf); VIR_FREE(replacedXML); VIR_FREE(xlcfgData); VIR_FREE(gotxml); diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index 3137dc564c..484b023a1c 100644 --- a/tests/xmconfigtest.c +++ b/tests/xmconfigtest.c @@ -41,7 +41,7 @@ static int testCompareParseXML(const char *xmcfg, const char *xml) { char *gotxmcfgData = NULL; - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL; int ret = -1; virConnectPtr conn = NULL; int wrote = 4096; @@ -76,8 +76,6 @@ testCompareParseXML(const char *xmcfg, const char *xml)

fail: VIR_FREE(gotxmcfgData); - if (conf) - virConfFree(conf); virDomainDefFree(def); virObjectUnref(conn);

@@ -89,7 +87,7 @@ testCompareFormatXML(const char *xmcfg, const char *xml) { char *xmcfgData = NULL; char *gotxml = NULL; - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL; int ret = -1; virDomainDefPtr def = NULL;

@@ -111,8 +109,6 @@ testCompareFormatXML(const char *xmcfg, const char *xml) ret = 0;

fail: - if (conf) - virConfFree(conf); VIR_FREE(xmcfgData); VIR_FREE(gotxml); virDomainDefFree(def); diff --git a/tools/virt-login-shell-helper.c b/tools/virt-login-shell-helper.c index d062c07a27..1a621ae53c 100644 --- a/tools/virt-login-shell-helper.c +++ b/tools/virt-login-shell-helper.c @@ -152,7 +152,7 @@ hideErrorFunc(void *opaque ATTRIBUTE_UNUSED, int main(int argc, char **argv) { - virConfPtr conf = NULL; + VIR_AUTOPTR(virConf) conf = NULL; const char *login_shell_path = conf_file; pid_t cpid = -1; int ret = EXIT_CANCELED; @@ -414,7 +414,6 @@ main(int argc, char **argv) for (i = 0; i < nfdlist; i++) VIR_FORCE_CLOSE(fdlist[i]); VIR_FREE(fdlist); - virConfFree(conf); if (dom) virDomainFree(dom); if (conn)