Re: [Qemu-devel] [PULL v2 13/32] qmp: Revert change to handle_qmp_command tracepoint

[Peter Maydell]

On 3 July 2018 at 22:35, Markus Armbruster <address@hidden> wrote:
> Commit 71da4667db6 "monitor: separate QMP parser and dispatcher" moved
> the handle_qmp_command tracepoint from handle_qmp_command() to
> monitor_qmp_dispatch_one().  This delays tracing from enqueue time to
> dequeue time.  Revert that.  Dequeue remains adequately visible via
> tracepoint monitor_qmp_cmd_in_band.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> Message-Id: <address@hidden>

Hi; it looks like this revert has reintroduced(?) a coverity
warning:

> ---
>  monitor.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 3bf3e68bdc..3cc4b07788 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4184,12 +4184,6 @@ static void monitor_qmp_dispatch_one(QMPRequest 
> *req_obj)
>
>      g_free(req_obj);
>
> -    if (trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
> -        QString *req_json = qobject_to_json(req);
> -        trace_handle_qmp_command(mon, qstring_get_str(req_json));
> -        qobject_unref(req_json);
> -    }
> -
>      old_mon = cur_mon;
>      cur_mon = mon;
>
> @@ -4283,6 +4277,12 @@ static void handle_qmp_command(JSONMessageParser 
> *parser, GQueue *tokens)
>          qdict_del(qdict, "id");
>      } /* else will fail qmp_dispatch() */
>
> +    if (trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
> +        QString *req_json = qobject_to_json(req);
> +        trace_handle_qmp_command(mon, qstring_get_str(req_json));
> +        qobject_unref(req_json);
> +    }

Coverity CID 1394216 points out that in this function a
little earlier we did a check on "if (!req && !err)", which
implies that we can get here with a NULL req pointer,
which will crash in qobject_to_json().

> +
>      /* Check against the request in general layout */
>      qdict = qmp_dispatch_check_obj(req, qmp_oob_enabled(mon), &err);
>      if (!qdict) {
> --

thanks
-- PMM