[libvirt] [PATCH v5 16/36] qemu_process: Cleanup qemuProcessQmp alloc function

[Jiri Denemark]

On Sun, Dec 02, 2018 at 23:10:10 -0600, Chris Venteicher wrote:
> qemuProcessQmpNew is one of the 4 public functions used to create and
> manage a qemu process for QMP command exchanges outside of domain
> operations.
> 
> Add descriptive comment block, Debug statement and make source
> consistent with the cleanup / VIR_STEAL_PTR format used elsewhere.
> 
> Signed-off-by: Chris Venteicher <cventeic at redhat.com>
> ---
>  src/qemu/qemu_process.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 31d41688fe..faf86dac5d 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -8124,6 +8124,18 @@ qemuProcessQmpFree(qemuProcessQmpPtr proc)
>  }
>  
>  
> +/**
> + * qemuProcessQmpNew:
> + * @binary: Qemu binary
> + * @libDir: Directory for process and connection artifacts
> + * @runUid: UserId for Qemu Process
> + * @runGid: GroupId for Qemu Process
> + * @forceTCG: Force TCG mode if true

s/Qemu/QEMU/

> + *
> + * Allocate and initialize domain structure encapsulating
> + * QEMU Process state and monitor connection to QEMU
> + * for completing QMP Queries.
> + */
>  qemuProcessQmpPtr
>  qemuProcessQmpNew(const char *binary,
>                    const char *libDir,
> @@ -8131,25 +8143,33 @@ qemuProcessQmpNew(const char *binary,
>                    gid_t runGid,
>                    bool forceTCG)
>  {
> +    qemuProcessQmpPtr ret = NULL;
>      qemuProcessQmpPtr proc = NULL;
>  
> +    VIR_DEBUG("exec=%s, libDir=%s, runUid=%u, runGid=%u, forceTCG=%d",
> +              NULLSTR(binary), NULLSTR(libDir), runUid, runGid, forceTCG);

The code is not designed to work with binary or libDir being NULL. Thus
we don't need to guard them here with NULLSTR() macro.

> +
>      if (VIR_ALLOC(proc) < 0)
> -        goto error;
> +        goto cleanup;
>  
>      if (VIR_STRDUP(proc->binary, binary) < 0 ||
>          VIR_STRDUP(proc->libDir, libDir) < 0)
> -        goto error;
> +        goto cleanup;
>  
>  
>      proc->runUid = runUid;
>      proc->runGid = runGid;
>      proc->forceTCG = forceTCG;
>  
> -    return proc;
> +    VIR_STEAL_PTR(ret, proc);
>  
> - error:
> -    qemuProcessQmpFree(proc);
> -    return NULL;
> + cleanup:
> +    if (proc)
> +        qemuProcessQmpFree(proc);

Just

       qemuProcessQmpFree(proc);

all *Free functions are designed to handle NULL pointers.

> +
> +    VIR_DEBUG("ret=%p", ret);
> +
> +    return ret;

This is the appropriate usage of VIR_STEAL_PTR() macro.

With the obvious s/Qmp/QMP/

Reviewed-by: Jiri Denemark <jdenemar at redhat.com>