On 05/03/2013 08:53 AM, Michal Privoznik wrote:
--- src/esx/esx_driver.c | 43 ++++++-------------- src/esx/esx_interface_driver.c | 7 +--- src/esx/esx_network_driver.c | 28 ++++--------- src/esx/esx_storage_backend_iscsi.c | 21 +++------- src/esx/esx_storage_backend_vmfs.c | 16 ++------ src/esx/esx_util.c | 45 +++++---------------- src/esx/esx_vi.c | 79 +++++++------------------------------ src/esx/esx_vi_types.c | 36 +++-------------- 8 files changed, 58 insertions(+), 217 deletions(-)
@@ -751,14 +747,9 @@ esxConnectToHost(esxPrivate *priv, VIR_WARN("The server is in maintenance mode"); }
- if (*vCenterIpAddress != NULL) { - *vCenterIpAddress = strdup(*vCenterIpAddress);
Old code: if string is not NULL, replace it with a copy from the heap...
- - if (*vCenterIpAddress == NULL) { - virReportOOMError(); - goto cleanup; - } - } + if (!*vCenterIpAddress && + VIR_STRDUP(*vCenterIpAddress, *vCenterIpAddress) < 0)
New code: if string is NULL, copy NULL to the heap. Oops. Drop the !
@@ -801,9 +792,7 @@ esxConnectToVCenter(esxPrivate *priv, }
if (conn->uri->user != NULL) { - username = strdup(conn->uri->user); - - if (username == NULL) { + if (VIR_STRDUP(username, conn->uri->user) < 0) { virReportOOMError();
Drop the oom error here.
@@ -1278,12 +1267,8 @@ esxConnectGetHostname(virConnectPtr conn) }
if (domainName == NULL || strlen(domainName) < 1) {
Pre-existing, but I'd have written the second condition as '!*domainName' (which is O(1) regardless of the length of domainName, whereas strlen()<1 is O(n) if the compiler doesn't optimize well)
@@ -1294,7 +1279,7 @@ esxConnectGetHostname(virConnectPtr conn) cleanup: /* * If we goto cleanup in case of an error then complete is still NULL, - * either strdup returned NULL or virAsprintf failed. When virAsprintf + * either VIR_STRDUP returned NULL or virAsprintf failed. When virAsprintf
Technically VIR_STRDUP returns -1, not NULL, if you want to touch up this comment.
+++ b/src/esx/esx_util.c @@ -171,23 +159,12 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri) } }
- if (uri->path != NULL) { - (*parsedUri)->path = strdup(uri->path); - - if ((*parsedUri)->path == NULL) { - virReportOOMError(); - goto cleanup; - } - } - - if ((*parsedUri)->transport == NULL) { - (*parsedUri)->transport = strdup("https"); + if (uri->path && VIR_STRDUP((*parsedUri)->path, uri->path) < 0) + goto cleanup;
Another argument in favor of allowing a NULL source argument.
+++ b/src/esx/esx_vi_types.c @@ -1327,16 +1310,7 @@ esxVI_String_DeserializeValue(xmlNodePtr node, char **value)
*value = (char *)xmlNodeListGetString(node->doc, node->children, 1);
- if (*value == NULL) { - *value = strdup("");
Old code: if xmlNodeListGetString found nothing, set *value to an empty string...
- - if (*value == NULL) { - virReportOOMError(); - return -1; - } - } - - return 0; + return value ? 0 : VIR_STRDUP(*value, "");
New code: since 'value' is non-NULL, unconditionally return 0 even if *value is still NULL. Oops. Missing a * ACK if you fix the bugs I called out. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org