4:07 p.m.
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
4:07 p.m.
New subject: [libvirt PATCH 1/9] esx: use g_autofree for char* where it is trivially possible
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(a)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
4:07 p.m.
New subject: [libvirt PATCH 2/9] esx: use g_autofree when made possible by reducing scope
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(a)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
4:07 p.m.
New subject: [libvirt PATCH 3/9] esx: fix memory leak by switching to g_autofree
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(a)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
4:07 p.m.
New subject: [libvirt PATCH 4/9] esx: switch VIR_FREE->g_free in esx*Free*()
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(a)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
4:07 p.m.
New subject: [libvirt PATCH 5/9] esx: use g_steal_pointer+g_autofree on return value
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(a)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
4:07 p.m.
New subject: [libvirt PATCH 6/9] esx: reorder code to avoid need to VIR_FREE mimeType
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(a)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
4:07 p.m.
New subject: [libvirt PATCH 7/9] esx: switch VIR_FREE->g_free when the pointer will immediately go out of scope
Or when it will be immediately have a new value assigned to it.
Signed-off-by: Laine Stump <laine(a)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
4:07 p.m.
New subject: [libvirt PATCH 8/9] esx: eliminate unnecessary cleanup: labels and result variables
switching to g_autofree left many cleanup: sections empty.
Signed-off-by: Laine Stump <laine(a)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
4:07 p.m.
New subject: [libvirt PATCH 9/9] esx: replace some VIR_FREE with g_clear_pointer(x, g_free)
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(a)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
4:17 a.m.
New subject: [libvirt PATCH 9/9] esx: replace some VIR_FREE with g_clear_pointer(x, g_free)
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(a)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
4:17 a.m.
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(a)redhat.com>
Michal
3:38 a.m.
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
3:44 a.m.
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
3:50 a.m.
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
1405
days inactive
1410
days old
14 comments
4 participants
participants (4)
-
Andrea Bolognani -
Ján Tomko -
Laine Stump -
Michal Privoznik