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

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



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 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.

Attachment: signature.asc
Description: Digital signature


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