[libvirt] [PATCH 3/3] qemu_monitor_json: Follow our coding style

Peter Krempa pkrempa at redhat.com
Tue May 3 11:33:14 UTC 2016


On Tue, May 03, 2016 at 11:53:21 +0200, Michal Privoznik wrote:
> In majority of our functions we have this variable @ret that is
> overwritten a lot. In other areas of the code we use 'goto
> cleanup;' just so that this wouldn't happen. But here.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_monitor_json.c | 295 ++++++++++++++++++++++---------------------
>  1 file changed, 150 insertions(+), 145 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 4dbf33f..47bb812 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c

[...]

> @@ -1278,12 +1283,16 @@ int qemuMonitorJSONSetLink(qemuMonitorPtr mon,
>      if (!cmd)
>          return -1;
>  
> -    if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) == 0)
> -        ret = qemuMonitorJSONCheckError(cmd, reply);
> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> +        goto cleanup;
>  
> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> + cleanup:

Your compiler should have complained that 'ret' might be uninitialized.

>      virJSONValueFree(cmd);
>      virJSONValueFree(reply);
> -
>      return ret;
>  }
>  
> @@ -1295,11 +1304,14 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon)
>      if (!cmd)
>          return -1;
>  
> -    ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> +        goto cleanup;
>  
> -    if (ret == 0)
> -        ret = qemuMonitorJSONCheckError(cmd, reply);
> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> +        goto cleanup;
>  
> +    ret = 0;

Here too.

> + cleanup:
>      virJSONValueFree(cmd);
>      virJSONValueFree(reply);
>      return ret;

[...]

> @@ -1847,7 +1847,6 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
>      }
>  
>      ret = 0;
> -

Spurious whitespace modification.

>   cleanup:
>      virJSONValueFree(cmd);
>      virJSONValueFree(reply);

I usually require a v2 for stuff that does not compile cleanly but I
don't really want to see this code again.

ACK if you fix the mistakes pointed out.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160503/e2d8caa9/attachment-0001.sig>


More information about the libvir-list mailing list