> 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