[libvirt] [PATCH v3 15/34] Adapt to VIR_STRDUP and VIR_STRNDUP in src/parallels/*

[Eric Blake]

On 05/03/2013 08:53 AM, Michal Privoznik wrote:
> ---
>  src/parallels/parallels_driver.c  | 71 +++++++++++++++++----------------------
>  src/parallels/parallels_network.c | 23 +++++--------
>  src/parallels/parallels_storage.c | 62 +++++++++++-----------------------
>  3 files changed, 58 insertions(+), 98 deletions(-)
> 
> diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
> @@ -209,8 +209,8 @@ parallelsGetSerialInfo(virDomainChrDefPtr chr,
>              return -1;
>          }
>  
> -        if (!(chr->source.data.file.path = strdup(tmp)))
> -            goto no_memory;
> +        if (VIR_STRDUP(chr->source.data.file.path, tmp) < 0)
> +            goto error;
>      } else {
>          parallelsParseError();
>          return -1;
> @@ -218,8 +218,7 @@ parallelsGetSerialInfo(virDomainChrDefPtr chr,
>  
>      return 0;
>  
> -  no_memory:
> -    virReportOOMError();
> +error:
>      return -1;

It looks a bit odd that this function has inlined 'return -1' in some
spots, and uses a label with no action other than 'return -1' in others.
 If you WANT to make it consistent, I don't care whether you use all
inline or all goto; but I'm not going to insist on consistency.

> @@ -771,13 +761,13 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj)
>      }
>  
>      if (STREQ(tmp, "CT")) {
> -        if (!(def->os.type = strdup("exe")))
> -            goto no_memory;
> -        if (!(def->os.init = strdup("/sbin/init")))
> -            goto no_memory;
> +        if (VIR_STRDUP(def->os.type, "exe") < 0)
> +            goto cleanup;
> +        if (VIR_STRDUP(def->os.init, "/sbin/init") < 0)
> +            goto cleanup;

You could merge these into one 'if'.

> +++ b/src/parallels/parallels_storage.c
> @@ -136,20 +136,9 @@ static char *parallelsMakePoolName(virConnectPtr conn, const char *path)
>          bool found = false;
>          int j;
>  
> -        if (!(name = strdup(path))) {
> -            virReportOOMError();
> -            return NULL;
> -        }
> -
> -        if (i == 0)
> -            name = strdup(path);

Thanks for squashing this memory leak in the process :)

> -        else
> -            ignore_value(virAsprintf(&name, "%s-%u", path, i));
> -
> -        if (!name) {
> -            virReportOOMError();
> +        if ((!i && VIR_STRDUP(name, path) < 0) ||
> +            (i && virAsprintf(&name, "%s-%u", path, i) < 0))
>              return 0;

Pre-existing, but this should be 'return NULL'.

> @@ -195,7 +184,8 @@ parallelsPoolCreateByPath(virConnectPtr conn, const char *path)
>      }
>  
>      def->type = VIR_STORAGE_POOL_DIR;
> -    def->target.path = strdup(path);
> +    if (VIR_STRDUP(def->target.path, path) < 0)
> +        goto error;

silent->noisy, but looks like a good bug fix.

> @@ -590,9 +579,9 @@ parallelsConnectListDefinedStoragePools(virConnectPtr conn,
>      for (i = 0; i < privconn->pools.count && n < nnames; i++) {
>          virStoragePoolObjLock(privconn->pools.objs[i]);
>          if (!virStoragePoolObjIsActive(privconn->pools.objs[i]) &&
> -            !(names[n++] = strdup(privconn->pools.objs[i]->def->name))) {
> +            VIR_STRDUP(names[n++], privconn->pools.objs[i]->def->name) < 0) {
>              virStoragePoolObjUnlock(privconn->pools.objs[i]);
> -            goto no_memory;
> +            goto error;
>          }
>          virStoragePoolObjUnlock(privconn->pools.objs[i]);
>      }
> @@ -600,7 +589,7 @@ parallelsConnectListDefinedStoragePools(virConnectPtr conn,
>  
>      return n;
>  
> -no_memory:
> +error:
>      virReportOOMError();

Drop this oom, since the only client of this label already reported oom.

ACK with minor fixes.

-- 
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/20130508/8a26458b/attachment-0001.sig>