[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v4 06/13] Adapt to VIR_STRDUP and VIR_STRNDUP in src/remote/*



On 05/20/2013 11:55 AM, Michal Privoznik wrote:
> ---
>  src/remote/remote_driver.c | 114 +++++++++++++++++----------------------------
>  1 file changed, 44 insertions(+), 70 deletions(-)
> 
> @@ -497,24 +497,21 @@ doRemoteOpen(virConnectPtr conn,
...
>  
>      if (conn->uri && conn->uri->user &&
> -        !(username = strdup(conn->uri->user)))
> -        goto no_memory;
> +        VIR_STRDUP(username, conn->uri->user) < 0)
> +        goto failed;

Could be simplified to:

if (conn->uri && VIR_STRDUP(username, conn->uri->user) < 0)

> @@ -705,17 +696,13 @@ doRemoteOpen(virConnectPtr conn,
>          break;
>  
>      case trans_ssh:
> -        if (!command && !(command = strdup("ssh")))
> -            goto no_memory;
> +        if (!command && VIR_STRDUP(command, "ssh") < 0)
> +            goto failed;
>  
> -        if (!sockname) {
> -            if (flags & VIR_DRV_OPEN_REMOTE_RO)
> -                sockname = strdup(LIBVIRTD_PRIV_UNIX_SOCKET_RO);
> -            else
> -                sockname = strdup(LIBVIRTD_PRIV_UNIX_SOCKET);
> -            if (!sockname)
> -                goto no_memory;
> -        }
> +        if (VIR_STRDUP(sockname,

Memory leak if sockname was already defined.  Must be:

if (!sockname && VIR_STRDUP(sockname, ...

> @@ -4472,15 +4449,15 @@ remoteDomainBuildEventGraphics(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
>      if (VIR_ALLOC(localAddr) < 0)
>          goto no_memory;
>      localAddr->family = msg->local.family;
> -    if (!(localAddr->service = strdup(msg->local.service)) ||
> -        !(localAddr->node = strdup(msg->local.node)))
> +    if (VIR_STRDUP(localAddr->service, msg->local.service) < 0 ||
> +        VIR_STRDUP(localAddr->node, msg->local.node) < 0)
>          goto no_memory;

silent->noisy, when we are really just discarding the event on any
error.  Based on discussion earlier in the series, that means we are
polluting the thread-local error object with no one to report it, but
that the pollution will be cleaned up on the next real API that we
handle.  So I think I can live with the change to noisy.  However,
s/no_memory/error/ would be appropriate.

ACK with those fixes.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]