Re: [Qemu-devel] [PATCH v2 06/18] qga: process_event() simplification and leak fix

[Michael Roth]

Quoting Marc-André Lureau (2018-07-19 13:40:59)
> json_parser_parse_err() may return something else than a QDict, in
> which case we loose the object. Let's keep track of the original
> object to avoid leaks.
> 
> When an error occurs, "qdict" contains the response, but we still
> check the "execute" key there. Untangle a bit this code, by having a
> clear error path.
> 
> CC: Michael Roth <address@hidden>
> Signed-off-by: Marc-André Lureau <address@hidden>
> Reviewed-by: Markus Armbruster <address@hidden>

Thanks, I've gone ahead and queued this one for 3.0/stable:
  https://github.com/mdroth/qemu/commits/qga

> ---
>  qga/main.c | 54 +++++++++++++++++++++++++++---------------------------
>  1 file changed, 27 insertions(+), 27 deletions(-)
> 
> diff --git a/qga/main.c b/qga/main.c
> index 537cc0e162..87372d40ef 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -600,42 +600,42 @@ static void process_command(GAState *s, QDict *req)
>  static void process_event(JSONMessageParser *parser, GQueue *tokens)
>  {
>      GAState *s = container_of(parser, GAState, parser);
> -    QDict *qdict;
> +    QObject *obj;
> +    QDict *req, *rsp;
>      Error *err = NULL;
>      int ret;
> 
>      g_assert(s && parser);
> 
>      g_debug("process_event: called");
> -    qdict = qobject_to(QDict, json_parser_parse_err(tokens, NULL, &err));
> -    if (err || !qdict) {
> -        qobject_unref(qdict);
> -        if (!err) {
> -            g_warning("failed to parse event: unknown error");
> -            error_setg(&err, QERR_JSON_PARSING);
> -        } else {
> -            g_warning("failed to parse event: %s", error_get_pretty(err));
> -        }
> -        qdict = qmp_error_response(err);
> +    obj = json_parser_parse_err(tokens, NULL, &err);
> +    if (err) {
> +        goto err;
>      }
> -
> -    /* handle host->guest commands */
> -    if (qdict_haskey(qdict, "execute")) {
> -        process_command(s, qdict);
> -    } else {
> -        if (!qdict_haskey(qdict, "error")) {
> -            qobject_unref(qdict);
> -            g_warning("unrecognized payload format");
> -            error_setg(&err, QERR_UNSUPPORTED);
> -            qdict = qmp_error_response(err);
> -        }
> -        ret = send_response(s, qdict);
> -        if (ret < 0) {
> -            g_warning("error sending error response: %s", strerror(-ret));
> -        }
> +    req = qobject_to(QDict, obj);
> +    if (!req) {
> +        error_setg(&err, QERR_JSON_PARSING);
> +        goto err;
> +    }
> +    if (!qdict_haskey(req, "execute")) {
> +        g_warning("unrecognized payload format");
> +        error_setg(&err, QERR_UNSUPPORTED);
> +        goto err;
>      }
> 
> -    qobject_unref(qdict);
> +    process_command(s, req);
> +    qobject_unref(obj);
> +    return;
> +
> +err:
> +    g_warning("failed to parse event: %s", error_get_pretty(err));
> +    rsp = qmp_error_response(err);
> +    ret = send_response(s, rsp);
> +    if (ret < 0) {
> +        g_warning("error sending error response: %s", strerror(-ret));
> +    }
> +    qobject_unref(rsp);
> +    qobject_unref(obj);
>  }
> 
>  /* false return signals GAChannel to close the current client connection */
> -- 
> 2.18.0.129.ge3331758f1
>