On Tue, May 03, 2016 at 11:53:19 +0200, Michal Privoznik wrote:
> Usually, the flow in this area of the code is as follows:
> 
> qemuMonitorJSONMakeCommand()
> qemuMonitorJSONCommand()
> qemuMonitorJSONCheckError()
> parseReply()
> 
> But in this function, for some reasons, the last two steps were
> swapped. This makes no sense.
> 
> Signed-off-by: Michal Privoznik <mpriv...@redhat.com>
> ---
>  src/qemu/qemu_monitor_json.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index a48a263..81970b9 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -3220,9 +3220,6 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
>      size_t i;
>      virNetDevRxFilterPtr fil = virNetDevRxFilterNew();
>  
> -    if (!fil)
> -        goto cleanup;
> -

The code dereferences 'fil' a few lines below. Without this check it
might crash.

>      if (!(returnArray = virJSONValueObjectGetArray(msg, "return"))) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                         _("query-rx-filter reply was missing return data"));
> @@ -3401,14 +3398,14 @@ qemuMonitorJSONQueryRxFilter(qemuMonitorPtr mon, 
> const char *alias,
>      if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
>          goto cleanup;
>  
> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> +        goto cleanup;
> +
>      if (qemuMonitorJSONQueryRxFilterParse(reply, filter) < 0)
>          goto cleanup;
>  
>      ret = 0;
>   cleanup:
> -    if (ret == 0)
> -        ret = qemuMonitorJSONCheckError(cmd, reply);
> -
>      if (ret < 0) {
>          virNetDevRxFilterFree(*filter);
>          *filter = NULL;

ACK to the last two hunks, the first one needs to be removed.

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to