[libvirt] [PATCH] esx: Remove redundant checks for esxVI_LookupHostSystemProperties result
esxVI_LookupHostSystemProperties guarantees that hostSystem is non-NULL. Remove redundant NULL checks from callers. Also prefer esxVI_GetStringValue over open-coding the logic. --- src/esx/esx_driver.c | 82 +++++++++--------------------------------------- src/esx/esx_vi.c | 4 +- src/esx/esx_vi_types.c | 3 +- 3 files changed, 19 insertions(+), 70 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 47957cc..72a7acc 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3,7 +3,7 @@ * esx_driver.c: core driver functions for managing VMware ESX hosts * * Copyright (C) 2010-2012 Red Hat, Inc. - * Copyright (C) 2009-2011 Matthias Bolte <matthias.bolte@googlemail.com> + * Copyright (C) 2009-2012 Matthias Bolte <matthias.bolte@googlemail.com> * Copyright (C) 2009 Maximilian Wilhelm <max@rfc2324.org> * * This library is free software; you can redistribute it and/or @@ -467,12 +467,6 @@ esxSupportsLongMode(esxPrivate *priv) goto cleanup; } - if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); - goto cleanup; - } - for (dynamicProperty = hostSystem->propSet; dynamicProperty != NULL; dynamicProperty = dynamicProperty->_next) { if (STREQ(dynamicProperty->name, "hardware.cpuFeature")) { @@ -534,7 +528,7 @@ esxLookupHostSystemBiosUuid(esxPrivate *priv, unsigned char *uuid) int result = -1; esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *hostSystem = NULL; - esxVI_DynamicProperty *dynamicProperty = NULL; + char *uuid_string = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -543,41 +537,22 @@ esxLookupHostSystemBiosUuid(esxPrivate *priv, unsigned char *uuid) if (esxVI_String_AppendValueToList(&propertyNameList, "hardware.systemInfo.uuid") < 0 || esxVI_LookupHostSystemProperties(priv->primary, propertyNameList, - &hostSystem) < 0) { - goto cleanup; - } - - if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); + &hostSystem) < 0 || + esxVI_GetStringValue(hostSystem, "hardware.systemInfo.uuid", + &uuid_string, esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; } - for (dynamicProperty = hostSystem->propSet; dynamicProperty != NULL; - dynamicProperty = dynamicProperty->_next) { - if (STREQ(dynamicProperty->name, "hardware.systemInfo.uuid")) { - if (esxVI_AnyType_ExpectType(dynamicProperty->val, - esxVI_Type_String) < 0) { - goto cleanup; - } - - if (strlen(dynamicProperty->val->string) > 0) { - if (virUUIDParse(dynamicProperty->val->string, uuid) < 0) { - VIR_WARN("Could not parse host UUID from string '%s'", - dynamicProperty->val->string); + if (strlen(uuid_string) > 0) { + if (virUUIDParse(uuid_string, uuid) < 0) { + VIR_WARN("Could not parse host UUID from string '%s'", uuid_string); - /* HostSystem has an invalid UUID, ignore it */ - memset(uuid, 0, VIR_UUID_BUFLEN); - } - } else { - /* HostSystem has an empty UUID */ - memset(uuid, 0, VIR_UUID_BUFLEN); - } - - break; - } else { - VIR_WARN("Unexpected '%s' property", dynamicProperty->name); + /* HostSystem has an invalid UUID, ignore it */ + memset(uuid, 0, VIR_UUID_BUFLEN); } + } else { + /* HostSystem has an empty UUID */ + memset(uuid, 0, VIR_UUID_BUFLEN); } result = 0; @@ -1176,17 +1151,8 @@ esxSupportsVMotion(esxPrivate *priv) if (esxVI_String_AppendValueToList(&propertyNameList, "capability.vmotionSupported") < 0 || esxVI_LookupHostSystemProperties(priv->primary, propertyNameList, - &hostSystem) < 0) { - goto cleanup; - } - - if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); - goto cleanup; - } - - if (esxVI_GetBoolean(hostSystem, "capability.vmotionSupported", + &hostSystem) < 0 || + esxVI_GetBoolean(hostSystem, "capability.vmotionSupported", &priv->supportsVMotion, esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; @@ -1281,12 +1247,6 @@ esxGetHostname(virConnectPtr conn) goto cleanup; } - if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); - goto cleanup; - } - for (dynamicProperty = hostSystem->propSet; dynamicProperty != NULL; dynamicProperty = dynamicProperty->_next) { if (STREQ(dynamicProperty->name, @@ -1379,12 +1339,6 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) goto cleanup; } - if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); - goto cleanup; - } - for (dynamicProperty = hostSystem->propSet; dynamicProperty != NULL; dynamicProperty = dynamicProperty->_next) { if (STREQ(dynamicProperty->name, "hardware.cpuInfo.hz")) { @@ -2702,12 +2656,6 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) goto cleanup; } - if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); - goto cleanup; - } - for (dynamicProperty = hostSystem->propSet; dynamicProperty != NULL; dynamicProperty = dynamicProperty->_next) { if (STREQ(dynamicProperty->name, "capability.maxSupportedVcpus")) { diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index c4217c9..f3a9e91 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -4570,8 +4570,8 @@ esxVI_ProductVersionToDefaultVirtualHWVersion(esxVI_ProductVersion productVersio esxVI_DynamicProperty *dynamicProperty = NULL; \ \ if (ptrptr == NULL || *ptrptr != NULL) { \ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ - _("Invalid argument")); \ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ + _("Invalid argument")); \ return -1; \ } \ \ diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index 36f3196..2236751 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -296,7 +296,8 @@ childNode = childNode->next) { \ if (childNode->type != XML_ELEMENT_NODE) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ - _("Wrong XML element type %d"), childNode->type); \ + _("Wrong XML element type %d"), \ + childNode->type); \ goto failure; \ } \ \ -- 1.7.4.1
Date: Sun, 5 Aug 2012 13:48:01 +0200 From: matthias.bolte@googlemail.com To: libvir-list@redhat.com Subject: [libvirt] [PATCH] esx: Remove redundant checks for esxVI_LookupHostSystemProperties result
esxVI_LookupHostSystemProperties guarantees that hostSystem is non-NULL. Remove redundant NULL checks from callers.
Also prefer esxVI_GetStringValue over open-coding the logic. --- src/esx/esx_driver.c | 82 +++++++++--------------------------------------- src/esx/esx_vi.c | 4 +- src/esx/esx_vi_types.c | 3 +- 3 files changed, 19 insertions(+), 70 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 47957cc..72a7acc 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3,7 +3,7 @@ * esx_driver.c: core driver functions for managing VMware ESX hosts * * Copyright (C) 2010-2012 Red Hat, Inc. - * Copyright (C) 2009-2011 Matthias Bolte <matthias.bolte@googlemail.com> + * Copyright (C) 2009-2012 Matthias Bolte <matthias.bolte@googlemail.com> * Copyright (C) 2009 Maximilian Wilhelm <max@rfc2324.org> * * This library is free software; you can redistribute it and/or @@ -467,12 +467,6 @@ esxSupportsLongMode(esxPrivate *priv) goto cleanup; }
- if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); - goto cleanup; - } - for (dynamicProperty = hostSystem->propSet; dynamicProperty != NULL; dynamicProperty = dynamicProperty->_next) { if (STREQ(dynamicProperty->name, "hardware.cpuFeature")) { @@ -534,7 +528,7 @@ esxLookupHostSystemBiosUuid(esxPrivate *priv, unsigned char *uuid) int result = -1; esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *hostSystem = NULL; - esxVI_DynamicProperty *dynamicProperty = NULL; + char *uuid_string = NULL;
if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -543,41 +537,22 @@ esxLookupHostSystemBiosUuid(esxPrivate *priv, unsigned char *uuid) if (esxVI_String_AppendValueToList(&propertyNameList, "hardware.systemInfo.uuid") < 0 || esxVI_LookupHostSystemProperties(priv->primary, propertyNameList, - &hostSystem) < 0) { - goto cleanup; - } - - if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); + &hostSystem) < 0 || + esxVI_GetStringValue(hostSystem, "hardware.systemInfo.uuid", + &uuid_string, esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; }
- for (dynamicProperty = hostSystem->propSet; dynamicProperty != NULL; - dynamicProperty = dynamicProperty->_next) { - if (STREQ(dynamicProperty->name, "hardware.systemInfo.uuid")) { - if (esxVI_AnyType_ExpectType(dynamicProperty->val, - esxVI_Type_String) < 0) { - goto cleanup; - } - - if (strlen(dynamicProperty->val->string) > 0) { - if (virUUIDParse(dynamicProperty->val->string, uuid) < 0) { - VIR_WARN("Could not parse host UUID from string '%s'", - dynamicProperty->val->string); + if (strlen(uuid_string) > 0) { + if (virUUIDParse(uuid_string, uuid) < 0) { + VIR_WARN("Could not parse host UUID from string '%s'", uuid_string);
- /* HostSystem has an invalid UUID, ignore it */ - memset(uuid, 0, VIR_UUID_BUFLEN); - } - } else { - /* HostSystem has an empty UUID */ - memset(uuid, 0, VIR_UUID_BUFLEN); - } - - break; - } else { - VIR_WARN("Unexpected '%s' property", dynamicProperty->name); + /* HostSystem has an invalid UUID, ignore it */ + memset(uuid, 0, VIR_UUID_BUFLEN); } + } else { + /* HostSystem has an empty UUID */ + memset(uuid, 0, VIR_UUID_BUFLEN); }
result = 0; @@ -1176,17 +1151,8 @@ esxSupportsVMotion(esxPrivate *priv) if (esxVI_String_AppendValueToList(&propertyNameList, "capability.vmotionSupported") < 0 || esxVI_LookupHostSystemProperties(priv->primary, propertyNameList, - &hostSystem) < 0) { - goto cleanup; - } - - if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); - goto cleanup; - } - - if (esxVI_GetBoolean(hostSystem, "capability.vmotionSupported", + &hostSystem) < 0 || + esxVI_GetBoolean(hostSystem, "capability.vmotionSupported", &priv->supportsVMotion, esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; @@ -1281,12 +1247,6 @@ esxGetHostname(virConnectPtr conn) goto cleanup; }
- if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); - goto cleanup; - } - for (dynamicProperty = hostSystem->propSet; dynamicProperty != NULL; dynamicProperty = dynamicProperty->_next) { if (STREQ(dynamicProperty->name, @@ -1379,12 +1339,6 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) goto cleanup; }
- if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); - goto cleanup; - } - for (dynamicProperty = hostSystem->propSet; dynamicProperty != NULL; dynamicProperty = dynamicProperty->_next) { if (STREQ(dynamicProperty->name, "hardware.cpuInfo.hz")) { @@ -2702,12 +2656,6 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) goto cleanup; }
- if (hostSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not retrieve the HostSystem object")); - goto cleanup; - } - for (dynamicProperty = hostSystem->propSet; dynamicProperty != NULL; dynamicProperty = dynamicProperty->_next) { if (STREQ(dynamicProperty->name, "capability.maxSupportedVcpus")) { diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index c4217c9..f3a9e91 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -4570,8 +4570,8 @@ esxVI_ProductVersionToDefaultVirtualHWVersion(esxVI_ProductVersion productVersio esxVI_DynamicProperty *dynamicProperty = NULL; \ \ if (ptrptr == NULL || *ptrptr != NULL) { \ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ - _("Invalid argument")); \ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ + _("Invalid argument")); \ return -1; \ } \ \ diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index 36f3196..2236751 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -296,7 +296,8 @@ childNode = childNode->next) { \ if (childNode->type != XML_ELEMENT_NODE) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ - _("Wrong XML element type %d"), childNode->type); \ + _("Wrong XML element type %d"), \ + childNode->type); \ goto failure; \ } \ \ -- 1.7.4.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
ACK, Thanks for the cleanup. Regards, Ata
On 05.08.2012 13:48, Matthias Bolte wrote:
esxVI_LookupHostSystemProperties guarantees that hostSystem is non-NULL. Remove redundant NULL checks from callers.
Also prefer esxVI_GetStringValue over open-coding the logic. --- src/esx/esx_driver.c | 82 +++++++++--------------------------------------- src/esx/esx_vi.c | 4 +- src/esx/esx_vi_types.c | 3 +- 3 files changed, 19 insertions(+), 70 deletions(-)
ACK Michal
2012/8/6 Michal Privoznik <mprivozn@redhat.com>:
On 05.08.2012 13:48, Matthias Bolte wrote:
esxVI_LookupHostSystemProperties guarantees that hostSystem is non-NULL. Remove redundant NULL checks from callers.
Also prefer esxVI_GetStringValue over open-coding the logic. --- src/esx/esx_driver.c | 82 +++++++++--------------------------------------- src/esx/esx_vi.c | 4 +- src/esx/esx_vi_types.c | 3 +- 3 files changed, 19 insertions(+), 70 deletions(-)
ACK
Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com
participants (3)
-
Ata Bohra -
Matthias Bolte -
Michal Privoznik