[libvirt] [PATCH -v2 0/2] esx: use coccinelle to avoid code duplication
Hi guys, in this second version I address the comments of Michal and Matthias. Changes from v1 are placed in each patch. Please let me know if you have more suggestions for the approach taken. Thanks, Marcos Paulo de Souza (2): esx_util.h: Add ESX_VI_CHECK_ARG_LIST macro esx: Use ESX_VI_CHECK_ARG_LIST macro to avoid code duplication src/esx/esx_driver.c | 5 +- src/esx/esx_network_driver.c | 10 +-- src/esx/esx_util.c | 5 +- src/esx/esx_util.h | 8 ++ src/esx/esx_vi.c | 165 +++++++---------------------------- src/esx/esx_vi_methods.c | 10 +-- src/esx/esx_vi_types.c | 51 +++-------- 7 files changed, 57 insertions(+), 197 deletions(-) -- 2.17.1
This macro avoids code duplication when checking for arrays of objects. Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> --- Changes from v1: * Change VIR_ERR_INVALID_ARG to VIR_ERR_INTERNAL_ERROR (Michal) * Change esxVI_checkArgList to ESX_VI_CHECK_ARG_LIST (Matthias) src/esx/esx_util.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h index c6f14bb2d9..63602bf3cb 100644 --- a/src/esx/esx_util.h +++ b/src/esx/esx_util.h @@ -26,6 +26,14 @@ # include "internal.h" # include "viruri.h" +#define ESX_VI_CHECK_ARG_LIST(val) \ + do { \ + if (!val || *val) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); \ + return -1; \ + } \ + } while (0) + typedef struct _esxUtil_ParsedUri esxUtil_ParsedUri; struct _esxUtil_ParsedUri { -- 2.17.1
On 07/04/2018 04:31 AM, Marcos Paulo de Souza wrote:
This macro avoids code duplication when checking for arrays of objects.
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> ---
Changes from v1: * Change VIR_ERR_INVALID_ARG to VIR_ERR_INTERNAL_ERROR (Michal) * Change esxVI_checkArgList to ESX_VI_CHECK_ARG_LIST (Matthias)
src/esx/esx_util.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h index c6f14bb2d9..63602bf3cb 100644 --- a/src/esx/esx_util.h +++ b/src/esx/esx_util.h @@ -26,6 +26,14 @@ # include "internal.h" # include "viruri.h"
+#define ESX_VI_CHECK_ARG_LIST(val) \
There needs to be a space between # and define (just line in context lines above). There has to be one space for every level of nesting.
+ do { \ + if (!val || *val) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); \ + return -1; \ + } \ + } while (0) + typedef struct _esxUtil_ParsedUri esxUtil_ParsedUri;
struct _esxUtil_ParsedUri {
Michal
By using this macro we can avoid boilerplate code to check for arrays of objects from ESX driver. This replacement was done using the coccinelle script bellow: @@ identifier ptr; @@ -if (!ptr || *ptr) { ... } +ESX_VI_CHECK_ARG_LIST(ptr); Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> --- Changes from v1: * Use ESX_VI_CHECK_ARG_LIST macro from previous patch * Use coccinelle script to check for all files inside esx directory src/esx/esx_driver.c | 5 +- src/esx/esx_network_driver.c | 10 +-- src/esx/esx_util.c | 5 +- src/esx/esx_vi.c | 165 +++++++---------------------------- src/esx/esx_vi_methods.c | 10 +-- src/esx/esx_vi_types.c | 51 +++-------- 6 files changed, 49 insertions(+), 197 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 01bcc99962..06e1238385 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -626,10 +626,7 @@ esxConnectToHost(esxPrivate *priv, ? esxVI_ProductLine_ESX : esxVI_ProductLine_GSX; - if (!vCenterIPAddress || *vCenterIPAddress) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(vCenterIPAddress); if (esxUtil_ResolveHostname(conn->uri->server, ipAddress, NI_MAXHOST) < 0) return -1; diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index b4f7f006d0..04118b4fa6 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -220,10 +220,7 @@ esxBandwidthToShapingPolicy(virNetDevBandwidthPtr bandwidth, { int result = -1; - if (!shapingPolicy || *shapingPolicy) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(shapingPolicy); if (!bandwidth->in || !bandwidth->out || bandwidth->in->average != bandwidth->out->average || @@ -589,10 +586,7 @@ static int esxShapingPolicyToBandwidth(esxVI_HostNetworkTrafficShapingPolicy *shapingPolicy, virNetDevBandwidthPtr *bandwidth) { - if (!bandwidth || *bandwidth) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(bandwidth); if (!shapingPolicy || shapingPolicy->enabled != esxVI_Boolean_True) return 0; diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index 2dd9f78569..d7210375fa 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -48,10 +48,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri) int autoAnswer; char *tmp; - if (!parsedUri || *parsedUri) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(parsedUri); if (VIR_ALLOC(*parsedUri) < 0) return -1; diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 25fbdc7e44..d3c4f760ba 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -51,10 +51,7 @@ VIR_LOG_INIT("esx.esx_vi"); int \ esxVI_##_type##_Alloc(esxVI_##_type **ptrptr) \ { \ - if (!ptrptr || *ptrptr) { \ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); \ - return -1; \ - } \ + ESX_VI_CHECK_ARG_LIST(ptrptr); \ \ if (VIR_ALLOC(*ptrptr) < 0) \ return -1; \ @@ -372,10 +369,7 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content, virBuffer buffer = VIR_BUFFER_INITIALIZER; int responseCode = 0; - if (!content || *content) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(content); if (length && *length > 0) { /* @@ -1762,10 +1756,7 @@ esxVI_List_DeepCopy(esxVI_List **destList, esxVI_List *srcList, esxVI_List *dest = NULL; esxVI_List *src = NULL; - if (!destList || *destList) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(destList); for (src = srcList; src; src = src->_next) { if (deepCopyFunc(&dest, src) < 0 || @@ -2170,10 +2161,7 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx, bool propertySpec_isAppended = false; esxVI_PropertyFilterSpec *propertyFilterSpec = NULL; - if (!objectContentList || *objectContentList) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(objectContentList); if (esxVI_ObjectSpec_Alloc(&objectSpec) < 0) return -1; @@ -2372,10 +2360,7 @@ esxVI_GetVirtualMachineQuestionInfo { esxVI_DynamicProperty *dynamicProperty; - if (!questionInfo || *questionInfo) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(questionInfo); for (dynamicProperty = virtualMachine->propSet; dynamicProperty; dynamicProperty = dynamicProperty->_next) { @@ -2447,10 +2432,7 @@ esxVI_GetInt(esxVI_ObjectContent *objectContent, const char *propertyName, { esxVI_DynamicProperty *dynamicProperty; - if (!value || *value) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(value); for (dynamicProperty = objectContent->propSet; dynamicProperty; dynamicProperty = dynamicProperty->_next) { @@ -2479,10 +2461,7 @@ esxVI_GetLong(esxVI_ObjectContent *objectContent, const char *propertyName, { esxVI_DynamicProperty *dynamicProperty; - if (!value || *value) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(value); for (dynamicProperty = objectContent->propSet; dynamicProperty; dynamicProperty = dynamicProperty->_next) { @@ -2512,10 +2491,7 @@ esxVI_GetStringValue(esxVI_ObjectContent *objectContent, { esxVI_DynamicProperty *dynamicProperty; - if (!value || *value) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(value); for (dynamicProperty = objectContent->propSet; dynamicProperty; dynamicProperty = dynamicProperty->_next) { @@ -2549,10 +2525,7 @@ esxVI_GetManagedObjectReference(esxVI_ObjectContent *objectContent, { esxVI_DynamicProperty *dynamicProperty; - if (!value || *value) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(value); for (dynamicProperty = objectContent->propSet; dynamicProperty; dynamicProperty = dynamicProperty->_next) { @@ -2863,10 +2836,7 @@ esxVI_GetSnapshotTreeBySnapshot { esxVI_VirtualMachineSnapshotTree *candidate; - if (!snapshotTree || *snapshotTree) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(snapshotTree); for (candidate = snapshotTreeList; candidate; candidate = candidate->_next) { @@ -2928,10 +2898,7 @@ esxVI_LookupVirtualMachineByUuid(esxVI_Context *ctx, const unsigned char *uuid, esxVI_ManagedObjectReference *managedObjectReference = NULL; char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; - if (!virtualMachine || *virtualMachine) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(virtualMachine); virUUIDFormat(uuid, uuid_string); @@ -2983,10 +2950,7 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name, esxVI_ObjectContent *candidate = NULL; char *name_candidate = NULL; - if (!virtualMachine || *virtualMachine) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(virtualMachine); if (esxVI_String_DeepCopyList(&completePropertyNameList, propertyNameList) < 0 || @@ -3110,10 +3074,7 @@ esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name, esxVI_ObjectContent *candidate = NULL; char *name_candidate; - if (!datastore || *datastore) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(datastore); /* Get all datastores */ if (esxVI_String_DeepCopyList(&completePropertyNameList, @@ -3177,10 +3138,7 @@ esxVI_LookupDatastoreByAbsolutePath(esxVI_Context *ctx, esxVI_DatastoreHostMount *datastoreHostMountList = NULL; esxVI_DatastoreHostMount *datastoreHostMount = NULL; - if (!datastore || *datastore) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(datastore); /* Get all datastores */ if (esxVI_String_DeepCopyList(&completePropertyNameList, @@ -3263,10 +3221,7 @@ esxVI_LookupDatastoreHostMount(esxVI_Context *ctx, esxVI_DatastoreHostMount *hostMountList = NULL; esxVI_DatastoreHostMount *candidate = NULL; - if (!hostMount || *hostMount) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(hostMount); if (esxVI_String_AppendValueToList(&propertyNameList, "host") < 0 || esxVI_LookupObjectContentByType(ctx, datastore, "Datastore", @@ -3327,10 +3282,7 @@ esxVI_LookupTaskInfoByTask(esxVI_Context *ctx, esxVI_ObjectContent *objectContent = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; - if (!taskInfo || *taskInfo) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(taskInfo); if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 || esxVI_LookupObjectContentByType(ctx, task, "Task", propertyNameList, @@ -3375,10 +3327,7 @@ esxVI_LookupPendingTaskInfoListByVirtualMachine esxVI_DynamicProperty *dynamicProperty = NULL; esxVI_TaskInfo *taskInfo = NULL; - if (!pendingTaskInfoList || *pendingTaskInfoList) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(pendingTaskInfoList); /* Get list of recent tasks */ for (dynamicProperty = virtualMachine->propSet; dynamicProperty; @@ -3481,10 +3430,7 @@ esxVI_LookupRootSnapshotTreeList esxVI_ObjectContent *virtualMachine = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; - if (!rootSnapshotTreeList || *rootSnapshotTreeList) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(rootSnapshotTreeList); if (esxVI_String_AppendValueToList(&propertyNameList, "snapshot.rootSnapshotList") < 0 || @@ -3536,10 +3482,7 @@ esxVI_LookupCurrentSnapshotTree esxVI_VirtualMachineSnapshotTree *rootSnapshotTreeList = NULL; esxVI_VirtualMachineSnapshotTree *snapshotTree = NULL; - if (!currentSnapshotTree || *currentSnapshotTree) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(currentSnapshotTree); if (esxVI_String_AppendValueListToList(&propertyNameList, "snapshot.currentSnapshot\0" @@ -3633,10 +3576,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, esxVI_TaskInfo *taskInfo = NULL; esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; - if (!fileInfo || *fileInfo) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(fileInfo); if (esxUtil_ParseDatastorePath(datastorePath, &datastoreName, &directoryName, &directoryAndFileName) < 0) { @@ -3829,10 +3769,7 @@ esxVI_LookupDatastoreContentByDatastoreName char *taskInfoErrorMessage = NULL; esxVI_TaskInfo *taskInfo = NULL; - if (!searchResultsList || *searchResultsList) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(searchResultsList); /* Lookup Datastore and HostDatastoreBrowser */ if (esxVI_String_AppendValueToList(&propertyNameList, "browser") < 0 || @@ -3941,10 +3878,7 @@ esxVI_LookupStorageVolumeKeyByDatastorePath(esxVI_Context *ctx, esxVI_FileInfo *fileInfo = NULL; char *uuid_string = NULL; - if (!key || *key) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(key); if (ctx->hasQueryVirtualDiskUuid) { if (esxVI_LookupFileInfoByDatastorePath @@ -3995,10 +3929,7 @@ esxVI_LookupAutoStartDefaults(esxVI_Context *ctx, esxVI_ObjectContent *hostAutoStartManager = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; - if (!defaults || *defaults) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(defaults); /* * Lookup HostAutoStartManagerConfig from the HostAutoStartManager because @@ -4052,10 +3983,7 @@ esxVI_LookupAutoStartPowerInfoList(esxVI_Context *ctx, esxVI_ObjectContent *hostAutoStartManager = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; - if (!powerInfoList || *powerInfoList) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(powerInfoList); /* * Lookup HostAutoStartManagerConfig from the HostAutoStartManager because @@ -4103,10 +4031,7 @@ esxVI_LookupPhysicalNicList(esxVI_Context *ctx, esxVI_ObjectContent *hostSystem = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; - if (!physicalNicList || *physicalNicList) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(physicalNicList); if (esxVI_String_AppendValueToList(&propertyNameList, "config.network.pnic") < 0 || @@ -4147,10 +4072,7 @@ esxVI_LookupPhysicalNicByName(esxVI_Context *ctx, const char *name, esxVI_PhysicalNic *physicalNicList = NULL; esxVI_PhysicalNic *candidate = NULL; - if (!physicalNic || *physicalNic) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(physicalNic); if (esxVI_LookupPhysicalNicList(ctx, &physicalNicList) < 0) goto cleanup; @@ -4194,10 +4116,7 @@ esxVI_LookupPhysicalNicByMACAddress(esxVI_Context *ctx, const char *mac, esxVI_PhysicalNic *physicalNicList = NULL; esxVI_PhysicalNic *candidate = NULL; - if (!physicalNic || *physicalNic) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(physicalNic); if (esxVI_LookupPhysicalNicList(ctx, &physicalNicList) < 0) goto cleanup; @@ -4241,10 +4160,7 @@ esxVI_LookupHostVirtualSwitchList(esxVI_Context *ctx, esxVI_ObjectContent *hostSystem = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; - if (!hostVirtualSwitchList || *hostVirtualSwitchList) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(hostVirtualSwitchList); if (esxVI_String_AppendValueToList(&propertyNameList, "config.network.vswitch") < 0 || @@ -4285,10 +4201,7 @@ esxVI_LookupHostVirtualSwitchByName(esxVI_Context *ctx, const char *name, esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL; esxVI_HostVirtualSwitch *candidate = NULL; - if (!hostVirtualSwitch || *hostVirtualSwitch) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(hostVirtualSwitch); if (esxVI_LookupHostVirtualSwitchList(ctx, &hostVirtualSwitchList) < 0) goto cleanup; @@ -4336,10 +4249,7 @@ esxVI_LookupHostPortGroupList(esxVI_Context *ctx, esxVI_ObjectContent *hostSystem = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; - if (!hostPortGroupList || *hostPortGroupList) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(hostPortGroupList); if (esxVI_String_AppendValueToList(&propertyNameList, "config.network.portgroup") < 0 || @@ -4513,10 +4423,7 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, bool blocked; esxVI_TaskInfo *taskInfo = NULL; - if (!errorMessage || *errorMessage) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(errorMessage); if (VIR_STRDUP(version, "") < 0) return -1; @@ -4982,10 +4889,7 @@ esxVI_LookupHostScsiTopologyLunListByTargetName bool found = false; esxVI_HostInternetScsiTargetTransport *candidate = NULL; - if (!hostScsiTopologyLunList || *hostScsiTopologyLunList) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(hostScsiTopologyLunList); if (esxVI_String_AppendValueToList (&propertyNameList, @@ -5076,10 +4980,7 @@ esxVI_LookupStoragePoolNameByScsiLunKey(esxVI_Context *ctx, esxVI_HostScsiTopologyLun *hostScsiTopologyLun; bool found = false; - if (!poolName || *poolName) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(poolName); if (esxVI_String_AppendValueToList (&propertyNameList, diff --git a/src/esx/esx_vi_methods.c b/src/esx/esx_vi_methods.c index a3d489dd66..3156a132d8 100644 --- a/src/esx/esx_vi_methods.c +++ b/src/esx/esx_vi_methods.c @@ -38,10 +38,7 @@ #define ESX_VI__METHOD__CHECK_OUTPUT__NotNone \ - if (!output || *output) { \ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); \ - return -1; \ - } + ESX_VI_CHECK_ARG_LIST(output); @@ -232,10 +229,7 @@ esxVI_RetrieveServiceContent(esxVI_Context *ctx, ESX_VI__SOAP__REQUEST_FOOTER; esxVI_Response *response = NULL; - if (!serviceContent || *serviceContent) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(serviceContent); if (esxVI_Context_Execute(ctx, "RetrieveServiceContent", request, &response, esxVI_Occurrence_RequiredItem) < 0 || diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index ec7fda9865..315d67d20f 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -43,10 +43,7 @@ VIR_LOG_INIT("esx.esx_vi_types"); int \ esxVI_##__type##_Alloc(esxVI_##__type **ptrptr) \ { \ - if (!ptrptr || *ptrptr) { \ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); \ - return -1; \ - } \ + ESX_VI_CHECK_ARG_LIST(ptrptr); \ \ if (VIR_ALLOC(*ptrptr) < 0) \ return -1; \ @@ -101,11 +98,7 @@ VIR_LOG_INIT("esx.esx_vi_types"); int \ esxVI_##_type##_DeepCopy(esxVI_##_type **dest, esxVI_##_type *src) \ { \ - if (!dest || *dest) { \ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ - _("Invalid argument")); \ - return -1; \ - } \ + ESX_VI_CHECK_ARG_LIST(dest); \ \ if (!src) { \ return 0; \ @@ -940,10 +933,7 @@ esxVI_AnyType_ExpectType(esxVI_AnyType *anyType, esxVI_Type type) int esxVI_AnyType_DeepCopy(esxVI_AnyType **dest, esxVI_AnyType *src) { - if (!dest || *dest) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(dest); if (!src) return 0; @@ -1009,10 +999,7 @@ esxVI_AnyType_Deserialize(xmlNodePtr node, esxVI_AnyType **anyType) { long long int number; - if (!anyType || *anyType) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(anyType); if (esxVI_AnyType_Alloc(anyType) < 0) return -1; @@ -1218,10 +1205,7 @@ ESX_VI__TEMPLATE__LIST__DEEP_COPY(String) int esxVI_String_DeepCopyValue(char **dest, const char *src) { - if (!dest || *dest) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(dest); if (!src) return 0; @@ -1270,10 +1254,7 @@ esxVI_String_SerializeValue(const char *value, const char *element, int esxVI_String_Deserialize(xmlNodePtr node, esxVI_String **string) { - if (!string || *string) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(string); if (esxVI_String_Alloc(string) < 0 || esxVI_String_DeserializeValue(node, &(*string)->value) < 0) { @@ -1294,10 +1275,7 @@ ESX_VI__TEMPLATE__LIST__DESERIALIZE(String) int esxVI_String_DeserializeValue(xmlNodePtr node, char **value) { - if (!value || *value) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(value); *value = (char *)xmlNodeListGetString(node->doc, node->children, 1); @@ -1472,10 +1450,7 @@ ESX_VI__TEMPLATE__SERIALIZE(DateTime, int esxVI_DateTime_Deserialize(xmlNodePtr node, esxVI_DateTime **dateTime) { - if (!dateTime || *dateTime) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(dateTime); if (esxVI_DateTime_Alloc(dateTime) < 0) return -1; @@ -1644,10 +1619,7 @@ ESX_VI__TEMPLATE__FREE(MethodFault, int esxVI_MethodFault_Deserialize(xmlNodePtr node, esxVI_MethodFault **methodFault) { - if (!methodFault || *methodFault) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(methodFault); if (esxVI_MethodFault_Alloc(methodFault) < 0) return -1; @@ -1738,10 +1710,7 @@ int esxVI_ManagedObjectReference_Deserialize (xmlNodePtr node, esxVI_ManagedObjectReference **managedObjectReference) { - if (!managedObjectReference || *managedObjectReference) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); - return -1; - } + ESX_VI_CHECK_ARG_LIST(managedObjectReference); if (esxVI_ManagedObjectReference_Alloc(managedObjectReference) < 0) return -1; -- 2.17.1
On 07/04/2018 04:31 AM, Marcos Paulo de Souza wrote:
Hi guys,
in this second version I address the comments of Michal and Matthias. Changes from v1 are placed in each patch. Please let me know if you have more suggestions for the approach taken.
Thanks,
Marcos Paulo de Souza (2): esx_util.h: Add ESX_VI_CHECK_ARG_LIST macro esx: Use ESX_VI_CHECK_ARG_LIST macro to avoid code duplication
src/esx/esx_driver.c | 5 +- src/esx/esx_network_driver.c | 10 +-- src/esx/esx_util.c | 5 +- src/esx/esx_util.h | 8 ++ src/esx/esx_vi.c | 165 +++++++---------------------------- src/esx/esx_vi_methods.c | 10 +-- src/esx/esx_vi_types.c | 51 +++-------- 7 files changed, 57 insertions(+), 197 deletions(-)
ACKed and pushed. Michal
participants (2)
-
Marcos Paulo de Souza -
Michal Prívozník