[libvirt PATCH 0/9] eliminate (almost) all VIR_FREE in esx directory
142 more down, 3084 to go :-) This goes through all the standard methods of eliminating VIR_FREE - first converting as many pointers as possible to g_autofree, then converting a few more *Free* functions (that were more questionable than previous Frees) to use g_free, then some trivial refactoring. And finally at the end a few stragglers that really do need the pointers cleared were changed to g_clear_pointer(x, g_free). A couple of issues: 1) There are two definitions of a macro called ESX_VI__TEMPLATE__FREE that generate functions with names in the form of "esxVI_.*_Free". These generated functions were doing "VIR_FREE(*ptrptr)" at the end; because of the "*", the caller's pointer would be cleared out (not just the local copy). There are something over 400 calls to these functions, and all but 10 or so that I audited will never reference the pointer after return from esxVI_Blah_Free(). But there are several that do. Since I'm unfamiliar with this code and don't want to risk breaking it, I opted to just use g_clear_pointer(ptrptr, g_free), thus preserving current behavior exactly. 2) There are still 7 instances of VIR_FREE in the esx directory. All 7 of them are in loops that are clearing out an array of names prior to returning failure, and from a quick glance it looks like there are at least a few places where it is important to clear the array entries. But rather than brute force convert to using g_clear_pointer in the loop, I figured someone may come up with an elegant translation to use GArray or GPtrArray instead, so I'm leaving them for now. Laine Stump (9): esx: use g_autofree for char* where it is trivially possible esx: use g_autofree when made possible by reducing scope esx: fix memory leak by switching to g_autofree esx: switch VIR_FREE->g_free in esx*Free*() esx: use g_steal_pointer+g_autofree on return value esx: reorder code to avoid need to VIR_FREE mimeType esx: switch VIR_FREE->g_free when the pointer will immediately go out of scope esx: eliminate unnecessary cleanup: labels and result variables esx: replace some VIR_FREE with g_clear_pointer(x, g_free) src/esx/esx_driver.c | 189 +++++++++------------------- src/esx/esx_network_driver.c | 4 +- src/esx/esx_storage_backend_iscsi.c | 16 +-- src/esx/esx_storage_backend_vmfs.c | 150 +++++++--------------- src/esx/esx_stream.c | 11 +- src/esx/esx_util.c | 41 +++--- src/esx/esx_vi.c | 111 ++++++---------- src/esx/esx_vi_methods.c | 3 +- src/esx/esx_vi_types.c | 18 +-- 9 files changed, 179 insertions(+), 364 deletions(-) -- 2.29.2
All of these strings are allocated once, freed once, and are never returned out of the function where they are created, used, and are freed. Signed-off-by: Laine Stump <laine@redhat.com> --- src/esx/esx_driver.c | 128 +++++++++-------------------- src/esx/esx_storage_backend_vmfs.c | 102 ++++++++--------------- src/esx/esx_stream.c | 7 +- src/esx/esx_util.c | 11 +-- src/esx/esx_vi.c | 53 ++++-------- src/esx/esx_vi_methods.c | 3 +- src/esx/esx_vi_types.c | 8 +- 7 files changed, 93 insertions(+), 219 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 0271f81a56..df257341b8 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -139,8 +139,8 @@ esxParseVMXFileName(const char *fileName, char *datastoreName; char *tmp; char *saveptr; - char *strippedFileName = NULL; - char *copyOfFileName = NULL; + g_autofree char *strippedFileName = NULL; + g_autofree char *copyOfFileName = NULL; char *directoryAndFileName; int ret = -1; @@ -253,8 +253,6 @@ esxParseVMXFileName(const char *fileName, esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&datastoreList); esxVI_DatastoreHostMount_Free(&hostMount); - VIR_FREE(strippedFileName); - VIR_FREE(copyOfFileName); return ret; } @@ -280,8 +278,8 @@ esxFormatVMXFileName(const char *fileName, void *opaque) bool success = false; char *result = NULL; esxVMX_Data *data = opaque; - char *datastoreName = NULL; - char *directoryAndFileName = NULL; + g_autofree char *datastoreName = NULL; + g_autofree char *directoryAndFileName = NULL; esxVI_ObjectContent *datastore = NULL; esxVI_DatastoreHostMount *hostMount = NULL; char separator = '/'; @@ -349,8 +347,6 @@ esxFormatVMXFileName(const char *fileName, void *opaque) if (! success) VIR_FREE(result); - VIR_FREE(datastoreName); - VIR_FREE(directoryAndFileName); esxVI_ObjectContent_Free(&datastore); esxVI_DatastoreHostMount_Free(&hostMount); @@ -613,9 +609,9 @@ esxConnectToHost(esxPrivate *priv, { int result = -1; g_autofree char *ipAddress = NULL; - char *username = NULL; - char *password = NULL; - char *url = NULL; + g_autofree char *username = NULL; + g_autofree char *password = NULL; + g_autofree char *url = NULL; esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *hostSystem = NULL; esxVI_Boolean inMaintenanceMode = esxVI_Boolean_Undefined; @@ -683,9 +679,6 @@ esxConnectToHost(esxPrivate *priv, result = 0; cleanup: - VIR_FREE(username); - VIR_FREE(password); - VIR_FREE(url); esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&hostSystem); @@ -703,9 +696,9 @@ esxConnectToVCenter(esxPrivate *priv, { int result = -1; g_autofree char *ipAddress = NULL; - char *username = NULL; - char *password = NULL; - char *url = NULL; + g_autofree char *username = NULL; + g_autofree char *password = NULL; + g_autofree char *url = NULL; if (!hostSystemIPAddress && (!priv->parsedUri->path || STREQ(priv->parsedUri->path, "/"))) { @@ -761,10 +754,6 @@ esxConnectToVCenter(esxPrivate *priv, result = 0; cleanup: - VIR_FREE(username); - VIR_FREE(password); - VIR_FREE(url); - return result; } @@ -822,7 +811,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, { virDrvOpenStatus result = VIR_DRV_OPEN_ERROR; esxPrivate *priv = NULL; - char *potentialVCenterIPAddress = NULL; + g_autofree char *potentialVCenterIPAddress = NULL; g_autofree char *vCenterIPAddress = NULL; virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); @@ -938,8 +927,6 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, cleanup: esxFreePrivate(&priv); - VIR_FREE(potentialVCenterIPAddress); - return result; } @@ -1472,7 +1459,7 @@ esxDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) esxVI_ObjectContent *virtualMachine = NULL; esxVI_VirtualMachinePowerState powerState; int id = -1; - char *name = NULL; + g_autofree char *name = NULL; virDomainPtr domain = NULL; if (esxVI_EnsureSession(priv->primary) < 0) @@ -1498,8 +1485,6 @@ esxDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) cleanup: esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&virtualMachine); - VIR_FREE(name); - return domain; } @@ -1559,7 +1544,7 @@ esxDomainSuspend(virDomainPtr domain) esxVI_VirtualMachinePowerState powerState; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; if (esxVI_EnsureSession(priv->primary) < 0) return -1; @@ -1599,8 +1584,6 @@ esxDomainSuspend(virDomainPtr domain) esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -1616,7 +1599,7 @@ esxDomainResume(virDomainPtr domain) esxVI_VirtualMachinePowerState powerState; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; if (esxVI_EnsureSession(priv->primary) < 0) return -1; @@ -1656,8 +1639,6 @@ esxDomainResume(virDomainPtr domain) esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -1767,7 +1748,7 @@ esxDomainDestroyFlags(virDomainPtr domain, esxVI_VirtualMachinePowerState powerState; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; virCheckFlags(0, -1); @@ -1816,8 +1797,6 @@ esxDomainDestroyFlags(virDomainPtr domain, esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -1902,7 +1881,7 @@ esxDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; if (esxVI_EnsureSession(priv->primary) < 0) return -1; @@ -1954,8 +1933,6 @@ esxDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -1972,7 +1949,7 @@ esxDomainSetMemoryFlags(virDomainPtr domain, esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; virCheckFlags(0, -1); @@ -2013,8 +1990,6 @@ esxDomainSetMemoryFlags(virDomainPtr domain, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -2434,7 +2409,7 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE, -1); @@ -2492,8 +2467,6 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -2574,14 +2547,14 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) esxVI_ObjectContent *virtualMachine = NULL; esxVI_VirtualMachinePowerState powerState; int id; - char *moref = NULL; + g_autofree char *moref = NULL; char *vmPathName = NULL; - char *datastoreName = NULL; - char *directoryName = NULL; - char *directoryAndFileName = NULL; + g_autofree char *datastoreName = NULL; + g_autofree char *directoryName = NULL; + g_autofree char *directoryAndFileName = NULL; g_auto(virBuffer) buffer = VIR_BUFFER_INITIALIZER; - char *url = NULL; - char *vmx = NULL; + g_autofree char *url = NULL; + g_autofree char *vmx = NULL; virVMXContext ctx; esxVMX_Data data; virDomainDefPtr def = NULL; @@ -2655,13 +2628,7 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) cleanup: esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&virtualMachine); - VIR_FREE(moref); - VIR_FREE(datastoreName); - VIR_FREE(directoryName); - VIR_FREE(directoryAndFileName); - VIR_FREE(url); VIR_FREE(data.datastorePathWithoutFileName); - VIR_FREE(vmx); virDomainDefFree(def); return xml; @@ -2858,7 +2825,7 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) int id = -1; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; virCheckFlags(0, -1); @@ -2903,7 +2870,6 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); return result; } @@ -2923,25 +2889,25 @@ esxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) { esxPrivate *priv = conn->privateData; virDomainDefPtr def = NULL; - char *vmx = NULL; + g_autofree char *vmx = NULL; size_t i; virDomainDiskDefPtr disk = NULL; esxVI_ObjectContent *virtualMachine = NULL; int virtualHW_version; virVMXContext ctx; esxVMX_Data data; - char *datastoreName = NULL; - char *directoryName = NULL; - char *escapedName = NULL; + g_autofree char *datastoreName = NULL; + g_autofree char *directoryName = NULL; + g_autofree char *escapedName = NULL; g_auto(virBuffer) buffer = VIR_BUFFER_INITIALIZER; - char *url = NULL; + g_autofree char *url = NULL; char *datastoreRelatedPath = NULL; esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *hostSystem = NULL; esxVI_ManagedObjectReference *resourcePool = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; virDomainPtr domain = NULL; const char *src; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; @@ -3122,19 +3088,11 @@ esxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) cleanup: virDomainDefFree(def); - VIR_FREE(vmx); - VIR_FREE(datastoreName); - VIR_FREE(directoryName); - VIR_FREE(escapedName); - VIR_FREE(url); - VIR_FREE(datastoreRelatedPath); esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&hostSystem); esxVI_ManagedObjectReference_Free(&resourcePool); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return domain; } @@ -3554,7 +3512,7 @@ esxDomainSetSchedulerParametersFlags(virDomainPtr domain, esxVI_SharesInfo *sharesInfo = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; size_t i; virCheckFlags(0, -1); @@ -3670,8 +3628,6 @@ esxDomainSetSchedulerParametersFlags(virDomainPtr domain, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -3735,7 +3691,7 @@ esxDomainMigratePerform(virDomainPtr domain, esxVI_Event *eventList = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; virCheckFlags(ESX_MIGRATION_FLAGS, -1); @@ -3850,8 +3806,6 @@ esxDomainMigratePerform(virDomainPtr domain, esxVI_ObjectContent_Free(&virtualMachine); esxVI_Event_Free(&eventList); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -4049,7 +4003,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_VirtualMachineSnapshotTree *snapshotTree = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; virDomainSnapshotPtr snapshot = NULL; bool diskOnly = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) != 0; bool quiesce = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) != 0; @@ -4122,8 +4076,6 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return snapshot; } @@ -4563,7 +4515,7 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) esxVI_VirtualMachineSnapshotTree *snapshotTree = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; virCheckFlags(0, -1); @@ -4599,8 +4551,6 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) cleanup: esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -4616,7 +4566,7 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) esxVI_Boolean removeChildren = esxVI_Boolean_False; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY, -1); @@ -4663,8 +4613,6 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) cleanup: esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -4680,7 +4628,7 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virTypedParameterPtr params, esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; size_t i; virCheckFlags(0, -1); @@ -4733,8 +4681,6 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virTypedParameterPtr params, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 27d8016194..9466ec81cb 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -672,8 +672,8 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, { virStorageVolPtr volume = NULL; esxPrivate *priv = pool->conn->privateData; - char *datastorePath = NULL; - char *key = NULL; + g_autofree char *datastorePath = NULL; + g_autofree char *key = NULL; datastorePath = g_strdup_printf("[%s] %s", pool->name, name); @@ -686,9 +686,6 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, &esxStorageBackendVMFS, NULL); cleanup: - VIR_FREE(datastorePath); - VIR_FREE(key); - return volume; } @@ -699,9 +696,9 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char *path) { virStorageVolPtr volume = NULL; esxPrivate *priv = conn->privateData; - char *datastoreName = NULL; - char *directoryAndFileName = NULL; - char *key = NULL; + g_autofree char *datastoreName = NULL; + g_autofree char *directoryAndFileName = NULL; + g_autofree char *key = NULL; if (esxUtil_ParseDatastorePath(path, &datastoreName, NULL, &directoryAndFileName) < 0) { @@ -717,10 +714,6 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char *path) &esxStorageBackendVMFS, NULL); cleanup: - VIR_FREE(datastoreName); - VIR_FREE(directoryAndFileName); - VIR_FREE(key); - return volume; } @@ -864,20 +857,20 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, esxPrivate *priv = pool->conn->privateData; virStoragePoolDef poolDef; char *tmp; - char *unescapedDatastorePath = NULL; - char *unescapedDirectoryName = NULL; - char *unescapedDirectoryAndFileName = NULL; - char *directoryName = NULL; - char *fileName = NULL; - char *datastorePathWithoutFileName = NULL; - char *datastorePath = NULL; + g_autofree char *unescapedDatastorePath = NULL; + g_autofree char *unescapedDirectoryName = NULL; + g_autofree char *unescapedDirectoryAndFileName = NULL; + g_autofree char *directoryName = NULL; + g_autofree char *fileName = NULL; + g_autofree char *datastorePathWithoutFileName = NULL; + g_autofree char *datastorePath = NULL; esxVI_FileInfo *fileInfo = NULL; esxVI_FileBackedVirtualDiskSpec *virtualDiskSpec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; - char *uuid_string = NULL; - char *key = NULL; + g_autofree char *taskInfoErrorMessage = NULL; + g_autofree char *uuid_string = NULL; + g_autofree char *key = NULL; g_autoptr(virStorageVolDef) def = NULL; virCheckFlags(0, NULL); @@ -1045,20 +1038,9 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, virtualDiskSpec->adapterType = NULL; } - VIR_FREE(unescapedDatastorePath); - VIR_FREE(unescapedDirectoryName); - VIR_FREE(unescapedDirectoryAndFileName); - VIR_FREE(directoryName); - VIR_FREE(fileName); - VIR_FREE(datastorePathWithoutFileName); - VIR_FREE(datastorePath); esxVI_FileInfo_Free(&fileInfo); esxVI_FileBackedVirtualDiskSpec_Free(&virtualDiskSpec); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - VIR_FREE(uuid_string); - VIR_FREE(key); - return volume; } @@ -1073,21 +1055,21 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, virStorageVolPtr volume = NULL; esxPrivate *priv = pool->conn->privateData; virStoragePoolDef poolDef; - char *sourceDatastorePath = NULL; + g_autofree char *sourceDatastorePath = NULL; char *tmp; - char *unescapedDatastorePath = NULL; - char *unescapedDirectoryName = NULL; - char *unescapedDirectoryAndFileName = NULL; - char *directoryName = NULL; - char *fileName = NULL; - char *datastorePathWithoutFileName = NULL; - char *datastorePath = NULL; + g_autofree char *unescapedDatastorePath = NULL; + g_autofree char *unescapedDirectoryName = NULL; + g_autofree char *unescapedDirectoryAndFileName = NULL; + g_autofree char *directoryName = NULL; + g_autofree char *fileName = NULL; + g_autofree char *datastorePathWithoutFileName = NULL; + g_autofree char *datastorePath = NULL; esxVI_FileInfo *fileInfo = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; - char *uuid_string = NULL; - char *key = NULL; + g_autofree char *taskInfoErrorMessage = NULL; + g_autofree char *uuid_string = NULL; + g_autofree char *key = NULL; g_autoptr(virStorageVolDef) def = NULL; virCheckFlags(0, NULL); @@ -1219,20 +1201,8 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, &esxStorageBackendVMFS, NULL); cleanup: - VIR_FREE(sourceDatastorePath); - VIR_FREE(unescapedDatastorePath); - VIR_FREE(unescapedDirectoryName); - VIR_FREE(unescapedDirectoryAndFileName); - VIR_FREE(directoryName); - VIR_FREE(fileName); - VIR_FREE(datastorePathWithoutFileName); - VIR_FREE(datastorePath); esxVI_FileInfo_Free(&fileInfo); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - VIR_FREE(uuid_string); - VIR_FREE(key); - return volume; } @@ -1243,10 +1213,10 @@ esxStorageVolDelete(virStorageVolPtr volume, unsigned int flags) { int result = -1; esxPrivate *priv = volume->conn->privateData; - char *datastorePath = NULL; + g_autofree char *datastorePath = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; virCheckFlags(0, -1); @@ -1271,10 +1241,7 @@ esxStorageVolDelete(virStorageVolPtr volume, unsigned int flags) result = 0; cleanup: - VIR_FREE(datastorePath); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -1285,10 +1252,10 @@ esxStorageVolWipe(virStorageVolPtr volume, unsigned int flags) { int result = -1; esxPrivate *priv = volume->conn->privateData; - char *datastorePath = NULL; + g_autofree char *datastorePath = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; virCheckFlags(0, -1); @@ -1313,10 +1280,7 @@ esxStorageVolWipe(virStorageVolPtr volume, unsigned int flags) result = 0; cleanup: - VIR_FREE(datastorePath); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); - return result; } @@ -1328,7 +1292,7 @@ esxStorageVolGetInfo(virStorageVolPtr volume, { int result = -1; esxPrivate *priv = volume->conn->privateData; - char *datastorePath = NULL; + g_autofree char *datastorePath = NULL; esxVI_FileInfo *fileInfo = NULL; esxVI_VmDiskFileInfo *vmDiskFileInfo = NULL; @@ -1358,7 +1322,6 @@ esxStorageVolGetInfo(virStorageVolPtr volume, result = 0; cleanup: - VIR_FREE(datastorePath); esxVI_FileInfo_Free(&fileInfo); return result; @@ -1372,7 +1335,7 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume, { esxPrivate *priv = volume->conn->privateData; virStoragePoolDef pool; - char *datastorePath = NULL; + g_autofree char *datastorePath = NULL; esxVI_FileInfo *fileInfo = NULL; esxVI_VmDiskFileInfo *vmDiskFileInfo = NULL; esxVI_IsoImageFileInfo *isoImageFileInfo = NULL; @@ -1438,7 +1401,6 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume, xml = virStorageVolDefFormat(&pool, &def); cleanup: - VIR_FREE(datastorePath); esxVI_FileInfo_Free(&fileInfo); VIR_FREE(def.key); diff --git a/src/esx/esx_stream.c b/src/esx/esx_stream.c index 2e7f979e79..cc48c182d9 100644 --- a/src/esx/esx_stream.c +++ b/src/esx/esx_stream.c @@ -395,8 +395,8 @@ esxStreamOpen(virStreamPtr stream, esxPrivate *priv, const char *url, { int result = -1; esxStreamPrivate *streamPriv; - char *range = NULL; - char *userpwd = NULL; + g_autofree char *range = NULL; + g_autofree char *userpwd = NULL; esxVI_MultiCURL *multi = NULL; /* FIXME: Although there is already some code in place to deal with @@ -467,9 +467,6 @@ esxStreamOpen(virStreamPtr stream, esxPrivate *priv, const char *url, esxFreeStreamPrivate(&streamPriv); } - VIR_FREE(range); - VIR_FREE(userpwd); - return result; } diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index d9e7641d67..64a2c968f0 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -207,7 +207,7 @@ esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName, char **directoryName, char **directoryAndFileName) { int result = -1; - char *copyOfDatastorePath = NULL; + g_autofree char *copyOfDatastorePath = NULL; char *tmp = NULL; char *saveptr = NULL; char *preliminaryDatastoreName = NULL; @@ -270,8 +270,6 @@ esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName, VIR_FREE(*directoryAndFileName); } - VIR_FREE(copyOfDatastorePath); - return result; } @@ -429,8 +427,8 @@ esxUtil_ReplaceSpecialWindowsPathChars(char *string) char * esxUtil_EscapeDatastoreItem(const char *string) { - char *replaced; - char *escaped1; + g_autofree char *replaced = NULL; + g_autofree char *escaped1 = NULL; char *escaped2 = NULL; replaced = g_strdup(string); @@ -445,9 +443,6 @@ esxUtil_EscapeDatastoreItem(const char *string) escaped2 = esxUtil_EscapeBase64(escaped1); cleanup: - VIR_FREE(replaced); - VIR_FREE(escaped1); - return escaped2; } diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 2eb8048858..7ff43adaaf 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -169,7 +169,7 @@ static int esxVI_CURL_Debug(CURL *curl G_GNUC_UNUSED, curl_infotype type, char *info, size_t size, void *userdata G_GNUC_UNUSED) { - char *buffer = NULL; + g_autofree char *buffer = NULL; /* * The libcurl documentation says: @@ -221,8 +221,6 @@ esxVI_CURL_Debug(CURL *curl G_GNUC_UNUSED, curl_infotype type, break; } - VIR_FREE(buffer); - return 0; } #endif @@ -833,7 +831,7 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, const char *password, esxUtil_ParsedUri *parsedUri) { int result = -1; - char *escapedPassword = NULL; + g_autofree char *escapedPassword = NULL; if (!ctx || !url || !ipAddress || !username || !password || ctx->url || ctx->service || ctx->curl) { @@ -965,8 +963,6 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, result = 0; cleanup: - VIR_FREE(escapedPassword); - return result; } @@ -1013,7 +1009,7 @@ int esxVI_Context_LookupManagedObjectsByPath(esxVI_Context *ctx, const char *path) { int result = -1; - char *tmp = NULL; + g_autofree char *tmp = NULL; char *saveptr = NULL; char *previousItem = NULL; char *item = NULL; @@ -1181,7 +1177,6 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context *ctx, const char *path) esxVI_ManagedObjectReference_Free(&root); } - VIR_FREE(tmp); esxVI_Folder_Free(&folder); return result; @@ -1239,7 +1234,7 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName, int result = -1; g_auto(virBuffer) buffer = VIR_BUFFER_INITIALIZER; esxVI_Fault *fault = NULL; - char *xpathExpression = NULL; + g_autofree char *xpathExpression = NULL; xmlXPathContextPtr xpathContext = NULL; xmlNodePtr responseNode = NULL; @@ -1401,7 +1396,6 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName, esxVI_Fault_Free(&fault); } - VIR_FREE(xpathExpression); xmlXPathFreeContext(xpathContext); return result; @@ -1509,7 +1503,7 @@ esxVI_Enumeration_Deserialize(const esxVI_Enumeration *enumeration, { size_t i; int result = -1; - char *name = NULL; + g_autofree char *name = NULL; if (!value) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); @@ -1534,8 +1528,6 @@ esxVI_Enumeration_Deserialize(const esxVI_Enumeration *enumeration, name, esxVI_Type_ToString(enumeration->type)); } - VIR_FREE(name); - return result; } @@ -1895,7 +1887,7 @@ esxVI_EnsureSession(esxVI_Context *ctx) esxVI_ObjectContent *sessionManager = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; esxVI_UserSession *currentSession = NULL; - char *escapedPassword = NULL; + g_autofree char *escapedPassword = NULL; if (!ctx->sessionLock) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call, no mutex")); @@ -1959,7 +1951,6 @@ esxVI_EnsureSession(esxVI_Context *ctx) cleanup: virMutexUnlock(ctx->sessionLock); - VIR_FREE(escapedPassword); esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&sessionManager); esxVI_UserSession_Free(¤tSession); @@ -3378,12 +3369,12 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, esxVI_Occurrence occurrence) { int result = -1; - char *datastoreName = NULL; - char *directoryName = NULL; - char *directoryAndFileName = NULL; - char *fileName = NULL; + g_autofree char *datastoreName = NULL; + g_autofree char *directoryName = NULL; + g_autofree char *directoryAndFileName = NULL; + g_autofree char *fileName = NULL; size_t length; - char *datastorePathWithoutFileName = NULL; + g_autofree char *datastorePathWithoutFileName = NULL; esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *datastore = NULL; esxVI_ManagedObjectReference *hostDatastoreBrowser = NULL; @@ -3394,7 +3385,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, esxVI_FloppyImageFileQuery *floppyImageFileQuery = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; esxVI_TaskInfo *taskInfo = NULL; esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; @@ -3544,17 +3535,11 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, if (searchSpec && searchSpec->matchPattern) searchSpec->matchPattern->value = NULL; - VIR_FREE(datastoreName); - VIR_FREE(directoryName); - VIR_FREE(directoryAndFileName); - VIR_FREE(fileName); - VIR_FREE(datastorePathWithoutFileName); esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&datastore); esxVI_ManagedObjectReference_Free(&hostDatastoreBrowser); esxVI_HostDatastoreBrowserSearchSpec_Free(&searchSpec); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); esxVI_TaskInfo_Free(&taskInfo); esxVI_HostDatastoreBrowserSearchResults_Free(&searchResults); esxVI_FolderFileQuery_Free(&folderFileQuery); @@ -3580,10 +3565,10 @@ esxVI_LookupDatastoreContentByDatastoreName esxVI_VmDiskFileQuery *vmDiskFileQuery = NULL; esxVI_IsoImageFileQuery *isoImageFileQuery = NULL; esxVI_FloppyImageFileQuery *floppyImageFileQuery = NULL; - char *datastorePath = NULL; + g_autofree char *datastorePath = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - char *taskInfoErrorMessage = NULL; + g_autofree char *taskInfoErrorMessage = NULL; esxVI_TaskInfo *taskInfo = NULL; ESX_VI_CHECK_ARG_LIST(searchResultsList); @@ -3672,9 +3657,7 @@ esxVI_LookupDatastoreContentByDatastoreName esxVI_ObjectContent_Free(&datastore); esxVI_ManagedObjectReference_Free(&hostDatastoreBrowser); esxVI_HostDatastoreBrowserSearchSpec_Free(&searchSpec); - VIR_FREE(datastorePath); esxVI_ManagedObjectReference_Free(&task); - VIR_FREE(taskInfoErrorMessage); esxVI_TaskInfo_Free(&taskInfo); esxVI_VmDiskFileQuery_Free(&vmDiskFileQuery); esxVI_IsoImageFileQuery_Free(&isoImageFileQuery); @@ -3692,7 +3675,7 @@ esxVI_LookupStorageVolumeKeyByDatastorePath(esxVI_Context *ctx, { int result = -1; esxVI_FileInfo *fileInfo = NULL; - char *uuid_string = NULL; + g_autofree char *uuid_string = NULL; ESX_VI_CHECK_ARG_LIST(key); @@ -3727,8 +3710,6 @@ esxVI_LookupStorageVolumeKeyByDatastorePath(esxVI_Context *ctx, cleanup: esxVI_FileInfo_Free(&fileInfo); - VIR_FREE(uuid_string); - return result; } @@ -4120,7 +4101,7 @@ esxVI_HandleVirtualMachineQuestion g_auto(virBuffer) buffer = VIR_BUFFER_INITIALIZER; esxVI_ElementDescription *answerChoice = NULL; int answerIndex = 0; - char *possibleAnswers = NULL; + g_autofree char *possibleAnswers = NULL; if (!blocked) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); @@ -4199,8 +4180,6 @@ esxVI_HandleVirtualMachineQuestion result = 0; cleanup: - VIR_FREE(possibleAnswers); - return result; } diff --git a/src/esx/esx_vi_methods.c b/src/esx/esx_vi_methods.c index e22a078997..87046b6225 100644 --- a/src/esx/esx_vi_methods.c +++ b/src/esx/esx_vi_methods.c @@ -106,7 +106,7 @@ int result = -1; \ const char *methodName = #_name; \ g_auto(virBuffer) buffer = VIR_BUFFER_INITIALIZER; \ - char *request = NULL; \ + g_autofree char *request = NULL; \ esxVI_Response *response = NULL; \ \ ESX_VI__METHOD__PARAMETER__THIS__##_this_from_service \ @@ -136,7 +136,6 @@ result = 0; \ \ cleanup: \ - VIR_FREE(request); \ esxVI_Response_Free(&response); \ \ return result; \ diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index 4d3617e0a8..1af075813e 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -330,7 +330,7 @@ VIR_LOG_INIT("esx.esx_vi_types"); esxVI_##_type##_Deserialize(xmlNodePtr node, esxVI_##_type **number) \ { \ int result = -1; \ - char *string; \ + g_autofree char *string = NULL; \ long long value; \ \ if (!number || *number) { \ @@ -374,8 +374,6 @@ VIR_LOG_INIT("esx.esx_vi_types"); if (result < 0) { \ esxVI_##_type##_Free(number); \ } \ - \ - VIR_FREE(string); \ \ return result; \ } @@ -703,7 +701,7 @@ esxVI_GetActualObjectType(xmlNodePtr node, esxVI_Type baseType, esxVI_Type *actualType) { int result = -1; - char *type = NULL; + g_autofree char *type = NULL; if (!actualType || *actualType != esxVI_Type_Undefined) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); @@ -732,8 +730,6 @@ esxVI_GetActualObjectType(xmlNodePtr node, esxVI_Type baseType, result = 0; cleanup: - VIR_FREE(type); - return result; } -- 2.29.2
These strings were being VIR_FREEd multiple times because they were defined at the top of a function, but then set each time through a loop. But they are only used inside that loop, so they can be converted to use g_autofree if their definition is also placed inside that loop. Signed-off-by: Laine Stump <laine@redhat.com> --- src/esx/esx_driver.c | 13 ++++--------- src/esx/esx_storage_backend_iscsi.c | 12 ++++-------- src/esx/esx_storage_backend_vmfs.c | 18 ++++-------------- src/esx/esx_vi.c | 5 +---- 4 files changed, 13 insertions(+), 35 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index df257341b8..8afec464dd 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1391,7 +1391,6 @@ esxDomainLookupByID(virConnectPtr conn, int id) esxVI_ObjectContent *virtualMachine = NULL; esxVI_VirtualMachinePowerState powerState; int id_candidate = -1; - char *name_candidate = NULL; unsigned char uuid_candidate[VIR_UUID_BUFLEN]; virDomainPtr domain = NULL; @@ -1410,6 +1409,8 @@ esxDomainLookupByID(virConnectPtr conn, int id) for (virtualMachine = virtualMachineList; virtualMachine; virtualMachine = virtualMachine->_next) { + g_autofree char *name_candidate = NULL; + if (esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0) { goto cleanup; @@ -1419,8 +1420,6 @@ esxDomainLookupByID(virConnectPtr conn, int id) if (powerState == esxVI_VirtualMachinePowerState_PoweredOff) continue; - VIR_FREE(name_candidate); - if (esxVI_GetVirtualMachineIdentity(virtualMachine, &id_candidate, &name_candidate, uuid_candidate) < 0) { @@ -1444,8 +1443,6 @@ esxDomainLookupByID(virConnectPtr conn, int id) cleanup: esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&virtualMachineList); - VIR_FREE(name_candidate); - return domain; } @@ -4754,7 +4751,6 @@ esxConnectListAllDomains(virConnectPtr conn, esxVI_AutoStartPowerInfo *powerInfoList = NULL; esxVI_AutoStartPowerInfo *powerInfo = NULL; esxVI_VirtualMachineSnapshotTree *rootSnapshotTreeList = NULL; - char *name = NULL; int id; unsigned char uuid[VIR_UUID_BUFLEN]; int count = 0; @@ -4839,9 +4835,9 @@ esxConnectListAllDomains(virConnectPtr conn, for (virtualMachine = virtualMachineList; virtualMachine; virtualMachine = virtualMachine->_next) { - if (needIdentity) { - VIR_FREE(name); + g_autofree char *name = NULL; + if (needIdentity) { if (esxVI_GetVirtualMachineIdentity(virtualMachine, &id, &name, uuid) < 0) { goto cleanup; @@ -4959,7 +4955,6 @@ esxConnectListAllDomains(virConnectPtr conn, VIR_FREE(doms); } - VIR_FREE(name); esxVI_AutoStartDefaults_Free(&autoStartDefaults); esxVI_AutoStartPowerInfo_Free(&powerInfoList); esxVI_String_Free(&propertyNameList); diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index 4d16ba2520..41bb9f6094 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -494,7 +494,6 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char *path) esxVI_ScsiLun *scsiLunList = NULL; esxVI_ScsiLun *scsiLun; esxVI_HostScsiDisk *hostScsiDisk = NULL; - char *poolName = NULL; /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; @@ -503,11 +502,12 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char *path) goto cleanup; for (scsiLun = scsiLunList; scsiLun; scsiLun = scsiLun->_next) { + g_autofree char *poolName = NULL; + hostScsiDisk = esxVI_HostScsiDisk_DynamicCast(scsiLun); if (hostScsiDisk && STREQ(hostScsiDisk->devicePath, path)) { /* Found matching device */ - VIR_FREE(poolName); if (esxVI_LookupStoragePoolNameByScsiLunKey(priv->primary, hostScsiDisk->key, @@ -527,8 +527,6 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char *path) cleanup: esxVI_ScsiLun_Free(&scsiLunList); - VIR_FREE(poolName); - return volume; } @@ -539,7 +537,6 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) { virStorageVolPtr volume = NULL; esxPrivate *priv = conn->privateData; - char *poolName = NULL; esxVI_ScsiLun *scsiLunList = NULL; esxVI_ScsiLun *scsiLun; /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ @@ -555,6 +552,8 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) for (scsiLun = scsiLunList; scsiLun; scsiLun = scsiLun->_next) { + g_autofree char *poolName = NULL; + memset(uuid_string, '\0', sizeof(uuid_string)); memset(md5, '\0', sizeof(md5)); @@ -564,7 +563,6 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) if (STREQ(key, uuid_string)) { /* Found matching UUID */ - VIR_FREE(poolName); if (esxVI_LookupStoragePoolNameByScsiLunKey(priv->primary, scsiLun->key, @@ -581,8 +579,6 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) cleanup: esxVI_ScsiLun_Free(&scsiLunList); - VIR_FREE(poolName); - return volume; } diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 9466ec81cb..63959ec237 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -598,7 +598,6 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char **const names, esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL; esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; esxVI_FileInfo *fileInfo = NULL; - char *directoryAndFileName = NULL; size_t length; int count = 0; size_t i; @@ -619,7 +618,7 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char **const names, /* Interpret search result */ for (searchResults = searchResultsList; searchResults; searchResults = searchResults->_next) { - VIR_FREE(directoryAndFileName); + g_autofree char *directoryAndFileName = NULL; if (esxUtil_ParseDatastorePath(searchResults->folderPath, NULL, NULL, &directoryAndFileName) < 0) { @@ -659,8 +658,6 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char **const names, } esxVI_HostDatastoreBrowserSearchResults_Free(&searchResultsList); - VIR_FREE(directoryAndFileName); - return count; } @@ -730,12 +727,9 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) char *datastoreName = NULL; esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL; esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; - char *directoryAndFileName = NULL; size_t length; - char *datastorePath = NULL; char *volumeName = NULL; esxVI_FileInfo *fileInfo = NULL; - char *uuid_string = NULL; char key_candidate[VIR_UUID_STRING_BUFLEN] = ""; if (STRPREFIX(key, "[")) { @@ -777,7 +771,7 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) /* Interpret search result */ for (searchResults = searchResultsList; searchResults; searchResults = searchResults->_next) { - VIR_FREE(directoryAndFileName); + g_autofree char *directoryAndFileName = NULL; if (esxUtil_ParseDatastorePath(searchResults->folderPath, NULL, NULL, &directoryAndFileName) < 0) { @@ -795,7 +789,8 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) /* Build datastore path and query the UUID */ for (fileInfo = searchResults->file; fileInfo; fileInfo = fileInfo->_next) { - VIR_FREE(datastorePath); + g_autofree char *datastorePath = NULL; + g_autofree char *uuid_string = NULL; if (length < 1) { volumeName = g_strdup(fileInfo->path); @@ -811,8 +806,6 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) continue; } - VIR_FREE(uuid_string); - if (esxVI_QueryVirtualDiskUuid (priv->primary, datastorePath, priv->primary->datacenter->_reference, @@ -838,10 +831,7 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&datastoreList); esxVI_HostDatastoreBrowserSearchResults_Free(&searchResultsList); - VIR_FREE(directoryAndFileName); - VIR_FREE(datastorePath); VIR_FREE(volumeName); - VIR_FREE(uuid_string); return volume; } diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 7ff43adaaf..987259be4b 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2761,7 +2761,6 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name, esxVI_String *completePropertyNameList = NULL; esxVI_ObjectContent *virtualMachineList = NULL; esxVI_ObjectContent *candidate = NULL; - char *name_candidate = NULL; ESX_VI_CHECK_ARG_LIST(virtualMachine); @@ -2775,7 +2774,7 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name, for (candidate = virtualMachineList; candidate; candidate = candidate->_next) { - VIR_FREE(name_candidate); + g_autofree char *name_candidate = NULL; if (esxVI_GetVirtualMachineIdentity(candidate, NULL, &name_candidate, NULL) < 0) { @@ -2802,8 +2801,6 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name, cleanup: esxVI_String_Free(&completePropertyNameList); esxVI_ObjectContent_Free(&virtualMachineList); - VIR_FREE(name_candidate); - return result; } -- 2.29.2
volumeName was defined at the top of the function, then a new string was assigned to it each time through a loop, but after the first iteration of the loop, the previous string wasn't freed before allocating a new string the next time. By reducing the scope of volumeName to be just the loop, and making it g_autofree, we eliminate the leak. Signed-off-by: Laine Stump <laine@redhat.com> --- src/esx/esx_storage_backend_vmfs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 63959ec237..225b2a4751 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -728,7 +728,6 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL; esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; size_t length; - char *volumeName = NULL; esxVI_FileInfo *fileInfo = NULL; char key_candidate[VIR_UUID_STRING_BUFLEN] = ""; @@ -789,6 +788,7 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) /* Build datastore path and query the UUID */ for (fileInfo = searchResults->file; fileInfo; fileInfo = fileInfo->_next) { + g_autofree char *volumeName = NULL; g_autofree char *datastorePath = NULL; g_autofree char *uuid_string = NULL; @@ -831,8 +831,6 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&datastoreList); esxVI_HostDatastoreBrowserSearchResults_Free(&searchResultsList); - VIR_FREE(volumeName); - return volume; } -- 2.29.2
Although the three functions esxFreePrivate(), esxFreeStreamPrivate(), and esxUtil_FreeParsedUri() are calling VIR_FREE on *object, and so in theory the caller of the function might rely on "object" (the free function's arg) being set to NULL, in practice these functions are only called from a couple places each, and in all cases the pointer that is passed is a local variable, and goes out of scope almost immediately after calling the Free function, so it is safe to change VIR_FREE() into g_free(). Signed-off-by: Laine Stump <laine@redhat.com> --- src/esx/esx_driver.c | 2 +- src/esx/esx_stream.c | 4 ++-- src/esx/esx_util.c | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 8afec464dd..952e769376 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -71,7 +71,7 @@ esxFreePrivate(esxPrivate **priv) esxUtil_FreeParsedUri(&(*priv)->parsedUri); virObjectUnref((*priv)->caps); virObjectUnref((*priv)->xmlopt); - VIR_FREE(*priv); + g_free(*priv); } diff --git a/src/esx/esx_stream.c b/src/esx/esx_stream.c index cc48c182d9..131fbc100b 100644 --- a/src/esx/esx_stream.c +++ b/src/esx/esx_stream.c @@ -336,8 +336,8 @@ esxFreeStreamPrivate(esxStreamPrivate **priv) return; esxVI_CURL_Free(&(*priv)->curl); - VIR_FREE((*priv)->backlog); - VIR_FREE(*priv); + g_free((*priv)->backlog); + g_free(*priv); } static int diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index 64a2c968f0..e9b74f386f 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -171,12 +171,12 @@ esxUtil_FreeParsedUri(esxUtil_ParsedUri **parsedUri) if (!parsedUri || !(*parsedUri)) return; - VIR_FREE((*parsedUri)->transport); - VIR_FREE((*parsedUri)->vCenter); - VIR_FREE((*parsedUri)->proxy_hostname); - VIR_FREE((*parsedUri)->path); + g_free((*parsedUri)->transport); + g_free((*parsedUri)->vCenter); + g_free((*parsedUri)->proxy_hostname); + g_free((*parsedUri)->path); - VIR_FREE(*parsedUri); + g_free(*parsedUri); } -- 2.29.2
If we put the potential return string into the g_autofreed tmpResult, and the move it to the returned "result" only as a final step ater, we can avoid the need to explicitly VIR_FREE (or g_free) on failure. Signed-off-by: Laine Stump <laine@redhat.com> --- src/esx/esx_driver.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 952e769376..47873c0d54 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -275,7 +275,7 @@ esxParseVMXFileName(const char *fileName, static char * esxFormatVMXFileName(const char *fileName, void *opaque) { - bool success = false; + g_autofree char *tmpResult = NULL; char *result = NULL; esxVMX_Data *data = opaque; g_autofree char *datastoreName = NULL; @@ -329,10 +329,10 @@ esxFormatVMXFileName(const char *fileName, void *opaque) virBufferAddChar(&buffer, separator); virBufferAdd(&buffer, directoryAndFileName, -1); - result = virBufferContentAndReset(&buffer); + tmpResult = virBufferContentAndReset(&buffer); } else if (*fileName == '/') { /* FIXME: need to deal with Windows paths here too */ - result = g_strdup(fileName); + tmpResult = g_strdup(fileName); } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not handle file name '%s'"), fileName); @@ -341,15 +341,11 @@ esxFormatVMXFileName(const char *fileName, void *opaque) /* FIXME: Check if referenced path/file really exists */ - success = true; + result = g_steal_pointer(&tmpResult); cleanup: - if (! success) - VIR_FREE(result); - esxVI_ObjectContent_Free(&datastore); esxVI_DatastoreHostMount_Free(&hostMount); - return result; } -- 2.29.2
mimeType is initialized to NULL, and then only set in one place, just before a check (not involving mimeType) that then VIR_FREEs mimeType if it fails. If we just reorder the code to do the check prior to setting mimeType, then there won't be any need to VIR_FREE(mimeType) on failure (because it will already be empty/NULL). Signed-off-by: Laine Stump <laine@redhat.com> --- src/esx/esx_driver.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 47873c0d54..2d010096a5 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2374,12 +2374,10 @@ esxDomainScreenshot(virDomainPtr domain, virStreamPtr stream, url = virBufferContentAndReset(&buffer); - mimeType = g_strdup("image/png"); - - if (esxStreamOpenDownload(stream, priv, url, 0, 0) < 0) { - VIR_FREE(mimeType); + if (esxStreamOpenDownload(stream, priv, url, 0, 0) < 0) goto cleanup; - } + + mimeType = g_strdup("image/png"); cleanup: -- 2.29.2
Or when it will be immediately have a new value assigned to it. Signed-off-by: Laine Stump <laine@redhat.com> --- src/esx/esx_driver.c | 4 ++-- src/esx/esx_network_driver.c | 2 +- src/esx/esx_storage_backend_iscsi.c | 4 ++-- src/esx/esx_storage_backend_vmfs.c | 4 ++-- src/esx/esx_util.c | 4 ++-- src/esx/esx_vi.c | 4 ++-- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 2d010096a5..fe98f90ea9 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2619,7 +2619,7 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) cleanup: esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&virtualMachine); - VIR_FREE(data.datastorePathWithoutFileName); + g_free(data.datastorePathWithoutFileName); virDomainDefFree(def); return xml; @@ -4946,7 +4946,7 @@ esxConnectListAllDomains(virConnectPtr conn, for (id = 0; id < count; id++) virObjectUnref(doms[id]); - VIR_FREE(doms); + g_free(doms); } esxVI_AutoStartDefaults_Free(&autoStartDefaults); diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 15fc7931c0..b489f4de8a 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -913,7 +913,7 @@ esxConnectListAllNetworks(virConnectPtr conn, if (ret < 0) { if (nets && *nets) { for (i = 0; i < count; ++i) - VIR_FREE((*nets)[i]); + g_free((*nets)[i]); VIR_FREE(*nets); } } diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index 41bb9f6094..d89b5a4ba8 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -348,7 +348,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) xml = virStoragePoolDefFormat(&def); cleanup: - VIR_FREE(def.source.hosts); + g_free(def.source.hosts); esxVI_HostInternetScsiHba_Free(&hostInternetScsiHba); return xml; @@ -727,7 +727,7 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume, cleanup: esxVI_ScsiLun_Free(&scsiLunList); - VIR_FREE(def.key); + g_free(def.key); return xml; } diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 225b2a4751..95505d6796 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -544,7 +544,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) xml = virStoragePoolDefFormat(&def); cleanup: - VIR_FREE(def.source.hosts); + g_free(def.source.hosts); esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&datastore); esxVI_DatastoreHostMount_Free(&hostMount); @@ -1390,7 +1390,7 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume, cleanup: esxVI_FileInfo_Free(&fileInfo); - VIR_FREE(def.key); + g_free(def.key); return xml; } diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index e9b74f386f..f1e8339fe0 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -55,7 +55,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri) virURIParamPtr queryParam = &uri->params[i]; if (STRCASEEQ(queryParam->name, "transport")) { - VIR_FREE((*parsedUri)->transport); + g_free((*parsedUri)->transport); (*parsedUri)->transport = g_strdup(queryParam->value); @@ -68,7 +68,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri) goto cleanup; } } else if (STRCASEEQ(queryParam->name, "vcenter")) { - VIR_FREE((*parsedUri)->vCenter); + g_free((*parsedUri)->vCenter); (*parsedUri)->vCenter = g_strdup(queryParam->value); } else if (STRCASEEQ(queryParam->name, "no_verify")) { diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 987259be4b..2a999f1cc1 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -4280,7 +4280,7 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, if (esxVI_WaitForUpdates(ctx, version, &updateSet) < 0) goto cleanup; - VIR_FREE(version); + g_free(version); version = g_strdup(updateSet->version); if (!updateSet->filterSet) @@ -4355,7 +4355,7 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, esxVI_PropertyFilterSpec_Free(&propertyFilterSpec); esxVI_ManagedObjectReference_Free(&propertyFilter); - VIR_FREE(version); + g_free(version); esxVI_UpdateSet_Free(&updateSet); esxVI_TaskInfo_Free(&taskInfo); -- 2.29.2
switching to g_autofree left many cleanup: sections empty. Signed-off-by: Laine Stump <laine@redhat.com> --- src/esx/esx_driver.c | 22 ++++++--------- src/esx/esx_storage_backend_vmfs.c | 22 +++++---------- src/esx/esx_util.c | 8 ++---- src/esx/esx_vi.c | 45 +++++++++++++----------------- src/esx/esx_vi_types.c | 8 ++---- 5 files changed, 38 insertions(+), 67 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index fe98f90ea9..e49975de86 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -690,7 +690,6 @@ esxConnectToVCenter(esxPrivate *priv, const char *hostname, const char *hostSystemIPAddress) { - int result = -1; g_autofree char *ipAddress = NULL; g_autofree char *username = NULL; g_autofree char *password = NULL; @@ -711,11 +710,11 @@ esxConnectToVCenter(esxPrivate *priv, } else { if (!(username = virAuthGetUsername(conn, auth, "esx", "administrator", hostname))) - goto cleanup; + return -1; } if (!(password = virAuthGetPassword(conn, auth, "esx", username, hostname))) - goto cleanup; + return -1; url = g_strdup_printf("%s://%s:%d/sdk", priv->parsedUri->transport, hostname, conn->uri->port); @@ -723,7 +722,7 @@ esxConnectToVCenter(esxPrivate *priv, if (esxVI_Context_Alloc(&priv->vCenter) < 0 || esxVI_Context_Connect(priv->vCenter, url, ipAddress, username, password, priv->parsedUri) < 0) { - goto cleanup; + return -1; } if (priv->vCenter->productLine != esxVI_ProductLine_VPX) { @@ -732,25 +731,20 @@ esxConnectToVCenter(esxPrivate *priv, hostname, esxVI_ProductLineToDisplayName(esxVI_ProductLine_VPX), esxVI_ProductLineToDisplayName(priv->vCenter->productLine)); - goto cleanup; + return -1; } if (hostSystemIPAddress) { - if (esxVI_Context_LookupManagedObjectsByHostSystemIp - (priv->vCenter, hostSystemIPAddress) < 0) { - goto cleanup; - } + if (esxVI_Context_LookupManagedObjectsByHostSystemIp(priv->vCenter, hostSystemIPAddress) < 0) + return -1; } else { if (esxVI_Context_LookupManagedObjectsByPath(priv->vCenter, priv->parsedUri->path) < 0) { - goto cleanup; + return -1; } } - result = 0; - - cleanup: - return result; + return 0; } diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 95505d6796..cb2be59a33 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -667,7 +667,6 @@ static virStorageVolPtr esxStorageVolLookupByName(virStoragePoolPtr pool, const char *name) { - virStorageVolPtr volume = NULL; esxPrivate *priv = pool->conn->privateData; g_autofree char *datastorePath = NULL; g_autofree char *key = NULL; @@ -676,14 +675,11 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, datastorePath, &key) < 0) { - goto cleanup; + return NULL; } - volume = virGetStorageVol(pool->conn, pool->name, name, key, - &esxStorageBackendVMFS, NULL); - - cleanup: - return volume; + return virGetStorageVol(pool->conn, pool->name, name, key, + &esxStorageBackendVMFS, NULL); } @@ -691,7 +687,6 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, static virStorageVolPtr esxStorageVolLookupByPath(virConnectPtr conn, const char *path) { - virStorageVolPtr volume = NULL; esxPrivate *priv = conn->privateData; g_autofree char *datastoreName = NULL; g_autofree char *directoryAndFileName = NULL; @@ -699,19 +694,16 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char *path) if (esxUtil_ParseDatastorePath(path, &datastoreName, NULL, &directoryAndFileName) < 0) { - goto cleanup; + return NULL; } if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, path, &key) < 0) { - goto cleanup; + return NULL; } - volume = virGetStorageVol(conn, datastoreName, directoryAndFileName, key, - &esxStorageBackendVMFS, NULL); - - cleanup: - return volume; + return virGetStorageVol(conn, datastoreName, directoryAndFileName, key, + &esxStorageBackendVMFS, NULL); } diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index f1e8339fe0..ef070a4f04 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -429,7 +429,6 @@ esxUtil_EscapeDatastoreItem(const char *string) { g_autofree char *replaced = NULL; g_autofree char *escaped1 = NULL; - char *escaped2 = NULL; replaced = g_strdup(string); @@ -438,12 +437,9 @@ esxUtil_EscapeDatastoreItem(const char *string) escaped1 = virVMXEscapeHexPercent(replaced); if (!escaped1) - goto cleanup; - - escaped2 = esxUtil_EscapeBase64(escaped1); + return NULL; - cleanup: - return escaped2; + return esxUtil_EscapeBase64(escaped1); } diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 2a999f1cc1..db5035c035 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -830,7 +830,6 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, const char *ipAddress, const char *username, const char *password, esxUtil_ParsedUri *parsedUri) { - int result = -1; g_autofree char *escapedPassword = NULL; if (!ctx || !url || !ipAddress || !username || @@ -844,12 +843,12 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, if (!escapedPassword) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to escape password for XML")); - goto cleanup; + return -1; } if (esxVI_CURL_Alloc(&ctx->curl) < 0 || esxVI_CURL_Connect(ctx->curl, parsedUri) < 0) { - goto cleanup; + return -1; } ctx->url = g_strdup(url); @@ -863,18 +862,18 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, if (virMutexInit(ctx->sessionLock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not initialize session mutex")); - goto cleanup; + return -1; } if (esxVI_RetrieveServiceContent(ctx, &ctx->service) < 0) - goto cleanup; + return -1; if (STRNEQ(ctx->service->about->apiType, "HostAgent") && STRNEQ(ctx->service->about->apiType, "VirtualCenter")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting VI API type 'HostAgent' or 'VirtualCenter' " "but found '%s'"), ctx->service->about->apiType); - goto cleanup; + return -1; } if (virParseVersionString(ctx->service->about->apiVersion, @@ -882,14 +881,14 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not parse VI API version '%s'"), ctx->service->about->apiVersion); - goto cleanup; + return -1; } if (ctx->apiVersion < 1000000 * 2 + 1000 * 5 /* 2.5 */) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Minimum supported %s version is %s but found version '%s'"), "VI API", "2.5", ctx->service->about->apiVersion); - goto cleanup; + return -1; } if (virParseVersionString(ctx->service->about->version, @@ -897,7 +896,7 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not parse product version '%s'"), ctx->service->about->version); - goto cleanup; + return -1; } if (STREQ(ctx->service->about->productLineId, "gsx")) { @@ -906,7 +905,7 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, _("Minimum supported %s version is %s but found version '%s'"), esxVI_ProductLineToDisplayName(esxVI_ProductLine_GSX), "2.0", ctx->service->about->version); - goto cleanup; + return -1; } ctx->productLine = esxVI_ProductLine_GSX; @@ -917,7 +916,7 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, _("Minimum supported %s version is %s but found version '%s'"), esxVI_ProductLineToDisplayName(esxVI_ProductLine_ESX), "3.5", ctx->service->about->version); - goto cleanup; + return -1; } ctx->productLine = esxVI_ProductLine_ESX; @@ -927,7 +926,7 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, _("Minimum supported %s version is %s but found version '%s'"), esxVI_ProductLineToDisplayName(esxVI_ProductLine_VPX), "2.5", ctx->service->about->version); - goto cleanup; + return -1; } ctx->productLine = esxVI_ProductLine_VPX; @@ -936,7 +935,7 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, _("Expecting product 'gsx' or 'esx' or 'embeddedEsx' " "or 'vpx' but found '%s'"), ctx->service->about->productLineId); - goto cleanup; + return -1; } if (ctx->productLine == esxVI_ProductLine_ESX) { @@ -957,13 +956,10 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, if (esxVI_Login(ctx, username, escapedPassword, NULL, &ctx->session) < 0 || esxVI_BuildSelectSetCollection(ctx) < 0) { - goto cleanup; + return -1; } - result = 0; - - cleanup: - return result; + return 0; } int @@ -4093,7 +4089,6 @@ esxVI_HandleVirtualMachineQuestion esxVI_VirtualMachineQuestionInfo *questionInfo, bool autoAnswer, bool *blocked) { - int result = -1; esxVI_ElementDescription *elementDescription = NULL; g_auto(virBuffer) buffer = VIR_BUFFER_INITIALIZER; esxVI_ElementDescription *answerChoice = NULL; @@ -4136,7 +4131,7 @@ esxVI_HandleVirtualMachineQuestion questionInfo->text); *blocked = true; - goto cleanup; + return -1; } else if (!answerChoice) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Pending question blocks virtual machine execution, " @@ -4145,7 +4140,7 @@ esxVI_HandleVirtualMachineQuestion possibleAnswers); *blocked = true; - goto cleanup; + return -1; } VIR_INFO("Pending question blocks virtual machine execution, " @@ -4155,7 +4150,7 @@ esxVI_HandleVirtualMachineQuestion if (esxVI_AnswerVM(ctx, virtualMachine, questionInfo->id, answerChoice->key) < 0) { - goto cleanup; + return -1; } } else { if (possibleAnswers) { @@ -4171,13 +4166,11 @@ esxVI_HandleVirtualMachineQuestion } *blocked = true; - goto cleanup; + return -1; } - result = 0; + return 0; - cleanup: - return result; } diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index 1af075813e..4dc7c30680 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -700,7 +700,6 @@ static int esxVI_GetActualObjectType(xmlNodePtr node, esxVI_Type baseType, esxVI_Type *actualType) { - int result = -1; g_autofree char *type = NULL; if (!actualType || *actualType != esxVI_Type_Undefined) { @@ -724,13 +723,10 @@ esxVI_GetActualObjectType(xmlNodePtr node, esxVI_Type baseType, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown value '%s' for %s 'type' property"), type, esxVI_Type_ToString(baseType)); - goto cleanup; + return -1; } - result = 0; - - cleanup: - return result; + return 0; } -- 2.29.2
These are all cases when 1) the pointer is passed by reference from the caller (ie.e. **) and expects it to be NULL on return if there is an error, or 2) the variable holding the pointer is being checked or re-used in the same function, but not right away. Signed-off-by: Laine Stump <laine@redhat.com> --- src/esx/esx_network_driver.c | 2 +- src/esx/esx_util.c | 8 ++++---- src/esx/esx_vi.c | 4 ++-- src/esx/esx_vi_types.c | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index b489f4de8a..4d0fba8c9f 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -914,7 +914,7 @@ esxConnectListAllNetworks(virConnectPtr conn, if (nets && *nets) { for (i = 0; i < count; ++i) g_free((*nets)[i]); - VIR_FREE(*nets); + g_clear_pointer(nets, g_free); } } diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index ef070a4f04..24e1c73ec4 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -95,7 +95,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri) /* Expected format: [<type>://]<hostname>[:<port>] */ (*parsedUri)->proxy = true; (*parsedUri)->proxy_type = CURLPROXY_HTTP; - VIR_FREE((*parsedUri)->proxy_hostname); + g_clear_pointer(&(*parsedUri)->proxy_hostname, g_free); (*parsedUri)->proxy_port = 1080; if ((tmp = STRSKIP(queryParam->value, "http://"))) { @@ -261,13 +261,13 @@ esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName, cleanup: if (result < 0) { if (datastoreName) - VIR_FREE(*datastoreName); + g_clear_pointer(datastoreName, g_free); if (directoryName) - VIR_FREE(*directoryName); + g_clear_pointer(directoryName, g_free); if (directoryAndFileName) - VIR_FREE(*directoryAndFileName); + g_clear_pointer(directoryAndFileName, g_free); } return result; diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index db5035c035..e1c1a15ab6 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -73,7 +73,7 @@ VIR_LOG_INIT("esx.esx_vi"); \ _body \ \ - VIR_FREE(*ptrptr); \ + g_clear_pointer(ptrptr, g_free); \ } @@ -2516,7 +2516,7 @@ esxVI_GetVirtualMachineIdentity(esxVI_ObjectContent *virtualMachine, failure: if (name) - VIR_FREE(*name); + g_clear_pointer(name, g_free); return -1; } diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index 4dc7c30680..59735194ae 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -67,7 +67,7 @@ VIR_LOG_INIT("esx.esx_vi_types"); \ _body \ \ - VIR_FREE(*ptrptr); \ + g_clear_pointer(ptrptr, g_free); \ } -- 2.29.2
On 2/12/21 11:07 PM, Laine Stump wrote:
These are all cases when 1) the pointer is passed by reference from the caller (ie.e. **) and expects it to be NULL on return if there is an error, or 2) the variable holding the pointer is being checked or re-used in the same function, but not right away.
Signed-off-by: Laine Stump <laine@redhat.com> --- src/esx/esx_network_driver.c | 2 +- src/esx/esx_util.c | 8 ++++---- src/esx/esx_vi.c | 4 ++-- src/esx/esx_vi_types.c | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index b489f4de8a..4d0fba8c9f 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -914,7 +914,7 @@ esxConnectListAllNetworks(virConnectPtr conn, if (nets && *nets) { for (i = 0; i < count; ++i) g_free((*nets)[i]); - VIR_FREE(*nets); + g_clear_pointer(nets, g_free);
This is one of the reasons I'm not fan of dropping our fine crafted utils and replace them with glib. But I don't want to stand in way to progress. Michal
On 2/12/21 11:07 PM, Laine Stump wrote:
142 more down, 3084 to go :-)
This goes through all the standard methods of eliminating VIR_FREE - first converting as many pointers as possible to g_autofree, then converting a few more *Free* functions (that were more questionable than previous Frees) to use g_free, then some trivial refactoring. And finally at the end a few stragglers that really do need the pointers cleared were changed to g_clear_pointer(x, g_free).
A couple of issues:
1) There are two definitions of a macro called ESX_VI__TEMPLATE__FREE that generate functions with names in the form of "esxVI_.*_Free". These generated functions were doing "VIR_FREE(*ptrptr)" at the end; because of the "*", the caller's pointer would be cleared out (not just the local copy). There are something over 400 calls to these functions, and all but 10 or so that I audited will never reference the pointer after return from esxVI_Blah_Free(). But there are several that do. Since I'm unfamiliar with this code and don't want to risk breaking it, I opted to just use g_clear_pointer(ptrptr, g_free), thus preserving current behavior exactly.
2) There are still 7 instances of VIR_FREE in the esx directory. All 7 of them are in loops that are clearing out an array of names prior to returning failure, and from a quick glance it looks like there are at least a few places where it is important to clear the array entries. But rather than brute force convert to using g_clear_pointer in the loop, I figured someone may come up with an elegant translation to use GArray or GPtrArray instead, so I'm leaving them for now.
Laine Stump (9): esx: use g_autofree for char* where it is trivially possible esx: use g_autofree when made possible by reducing scope esx: fix memory leak by switching to g_autofree esx: switch VIR_FREE->g_free in esx*Free*() esx: use g_steal_pointer+g_autofree on return value esx: reorder code to avoid need to VIR_FREE mimeType esx: switch VIR_FREE->g_free when the pointer will immediately go out of scope esx: eliminate unnecessary cleanup: labels and result variables esx: replace some VIR_FREE with g_clear_pointer(x, g_free)
src/esx/esx_driver.c | 189 +++++++++------------------- src/esx/esx_network_driver.c | 4 +- src/esx/esx_storage_backend_iscsi.c | 16 +-- src/esx/esx_storage_backend_vmfs.c | 150 +++++++--------------- src/esx/esx_stream.c | 11 +- src/esx/esx_util.c | 41 +++--- src/esx/esx_vi.c | 111 ++++++---------- src/esx/esx_vi_methods.c | 3 +- src/esx/esx_vi_types.c | 18 +-- 9 files changed, 179 insertions(+), 364 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On Fri, 2021-02-12 at 17:07 -0500, Laine Stump wrote:
Laine Stump (9): esx: use g_autofree for char* where it is trivially possible esx: use g_autofree when made possible by reducing scope esx: fix memory leak by switching to g_autofree esx: switch VIR_FREE->g_free in esx*Free*() esx: use g_steal_pointer+g_autofree on return value esx: reorder code to avoid need to VIR_FREE mimeType esx: switch VIR_FREE->g_free when the pointer will immediately go out of scope esx: eliminate unnecessary cleanup: labels and result variables esx: replace some VIR_FREE with g_clear_pointer(x, g_free)
src/esx/esx_driver.c | 189 +++++++++------------------- src/esx/esx_network_driver.c | 4 +- src/esx/esx_storage_backend_iscsi.c | 16 +-- src/esx/esx_storage_backend_vmfs.c | 150 +++++++--------------- src/esx/esx_stream.c | 11 +- src/esx/esx_util.c | 41 +++--- src/esx/esx_vi.c | 111 ++++++---------- src/esx/esx_vi_methods.c | 3 +- src/esx/esx_vi_types.c | 18 +-- 9 files changed, 179 insertions(+), 364 deletions(-)
This appears to have broken building with Clang on Linux: https://gitlab.com/libvirt/libvirt/-/pipelines/257175967 Can you please look into the issue? -- Andrea Bolognani / Red Hat / Virtualization
On a Wednesday in 2021, Andrea Bolognani wrote:
On Fri, 2021-02-12 at 17:07 -0500, Laine Stump wrote:
Laine Stump (9): esx: use g_autofree for char* where it is trivially possible esx: use g_autofree when made possible by reducing scope esx: fix memory leak by switching to g_autofree esx: switch VIR_FREE->g_free in esx*Free*() esx: use g_steal_pointer+g_autofree on return value esx: reorder code to avoid need to VIR_FREE mimeType esx: switch VIR_FREE->g_free when the pointer will immediately go out of scope esx: eliminate unnecessary cleanup: labels and result variables esx: replace some VIR_FREE with g_clear_pointer(x, g_free)
src/esx/esx_driver.c | 189 +++++++++------------------- src/esx/esx_network_driver.c | 4 +- src/esx/esx_storage_backend_iscsi.c | 16 +-- src/esx/esx_storage_backend_vmfs.c | 150 +++++++--------------- src/esx/esx_stream.c | 11 +- src/esx/esx_util.c | 41 +++--- src/esx/esx_vi.c | 111 ++++++---------- src/esx/esx_vi_methods.c | 3 +- src/esx/esx_vi_types.c | 18 +-- 9 files changed, 179 insertions(+), 364 deletions(-)
This appears to have broken building with Clang on Linux:
https://gitlab.com/libvirt/libvirt/-/pipelines/257175967
Can you please look into the issue?
It should be fixed by 1f8e6a6172f2df1c4eab1cf2a26488498947641f Jano
-- Andrea Bolognani / Red Hat / Virtualization
On Wed, 2021-02-17 at 10:44 +0100, Ján Tomko wrote:
On a Wednesday in 2021, Andrea Bolognani wrote:
On Fri, 2021-02-12 at 17:07 -0500, Laine Stump wrote:
Laine Stump (9): esx: use g_autofree for char* where it is trivially possible esx: use g_autofree when made possible by reducing scope esx: fix memory leak by switching to g_autofree esx: switch VIR_FREE->g_free in esx*Free*() esx: use g_steal_pointer+g_autofree on return value esx: reorder code to avoid need to VIR_FREE mimeType esx: switch VIR_FREE->g_free when the pointer will immediately go out of scope esx: eliminate unnecessary cleanup: labels and result variables esx: replace some VIR_FREE with g_clear_pointer(x, g_free)
src/esx/esx_driver.c | 189 +++++++++------------------- src/esx/esx_network_driver.c | 4 +- src/esx/esx_storage_backend_iscsi.c | 16 +-- src/esx/esx_storage_backend_vmfs.c | 150 +++++++--------------- src/esx/esx_stream.c | 11 +- src/esx/esx_util.c | 41 +++--- src/esx/esx_vi.c | 111 ++++++---------- src/esx/esx_vi_methods.c | 3 +- src/esx/esx_vi_types.c | 18 +-- 9 files changed, 179 insertions(+), 364 deletions(-)
This appears to have broken building with Clang on Linux:
https://gitlab.com/libvirt/libvirt/-/pipelines/257175967
Can you please look into the issue?
It should be fixed by 1f8e6a6172f2df1c4eab1cf2a26488498947641f
So it is! Nevermind then :) -- Andrea Bolognani / Red Hat / Virtualization
participants (4)
-
Andrea Bolognani -
Ján Tomko -
Laine Stump -
Michal Privoznik