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