[libvirt] [PATCH v3 05/34] Adapt to VIR_STRDUP and VIR_STRNDUP in src/esx/*

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130506/6d71acf2/attachment-0001.sig>