Copying the maintainer Michael Roth.
Marc-André Lureau <marcandre.lur...@redhat.com> writes:
> Simplify the code around qmp_dispatch():
> - rely on qmp_dispatch/check_obj() for message checking
> - have a single send_response() point
> - constify send_response() argument
> - rsp/req variable renaming for clarity
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> qga/main.c | 58 ++++++++++++++----------------------------------------
> 1 file changed, 15 insertions(+), 43 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 0784761605..b0a88ee5cf 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -545,15 +545,15 @@ fail:
> #endif
> }
>
> -static int send_response(GAState *s, QDict *payload)
> +static int send_response(GAState *s, const QDict *rsp)
> {
> const char *buf;
> QString *payload_qstr, *response_qstr;
> GIOStatus status;
>
> - g_assert(payload && s->channel);
> + g_assert(rsp && s->channel);
>
> - payload_qstr = qobject_to_json(QOBJECT(payload));
> + payload_qstr = qobject_to_json(QOBJECT(rsp));
> if (!payload_qstr) {
> return -EINVAL;
> }
> @@ -579,63 +579,35 @@ static int send_response(GAState *s, QDict *payload)
> return 0;
> }
>
> -static void process_command(GAState *s, QDict *req)
> -{
> - QDict *rsp;
> - int ret;
> -
> - g_assert(req);
> - g_debug("processing command");
> - rsp = qmp_dispatch(&ga_commands, QOBJECT(req), false);
> - if (rsp) {
> - ret = send_response(s, rsp);
> - if (ret < 0) {
> - g_warning("error sending response: %s", strerror(-ret));
> - }
> - qobject_unref(rsp);
> - }
> -}
> -
> /* handle requests/control events coming in over the channel */
> static void process_event(JSONMessageParser *parser, GQueue *tokens)
> {
> GAState *s = container_of(parser, GAState, parser);
> - QObject *obj;
> - QDict *qdict;
> + QObject *req;
> + QDict *rsp = NULL;
> Error *err = NULL;
> int ret;
>
> g_assert(s && parser);
>
> g_debug("process_event: called");
> - obj = json_parser_parse_err(tokens, NULL, &err);
> +
> + req = json_parser_parse_err(tokens, NULL, &err);
> if (err) {
> - goto err;
> - }
> - qdict = qobject_to(QDict, obj);
> - if (!qdict) {
> - error_setg(&err, QERR_JSON_PARSING);
> - goto err;
> - }
> - if (!qdict_haskey(qdict, "execute")) {
> - g_warning("unrecognized payload format");
> - error_setg(&err, QERR_UNSUPPORTED);
> - goto err;
> + rsp = qmp_error_response(err);
> + goto end;
> }
>
> - process_command(s, qdict);
> - qobject_unref(obj);
> - return;
> + g_debug("processing command");
> + rsp = qmp_dispatch(&ga_commands, req, false);
>
> -err:
> - g_warning("failed to parse event: %s", error_get_pretty(err));
> - qdict = qmp_error_response(err);
> - ret = send_response(s, qdict);
> +end:
> + ret = send_response(s, rsp);
> if (ret < 0) {
> g_warning("error sending error response: %s", strerror(-ret));
> }
> - qobject_unref(qdict);
> - qobject_unref(obj);
> + qobject_unref(rsp);
> + qobject_unref(req);
> }
>
> /* false return signals GAChannel to close the current client connection */
Nice. It changes a couple of error messages:
* When @req isn't a dictionary, from
Invalid JSON syntax
to
QMP input must be a JSON object
* When @req lacks member "execute", from
this feature or command is not currently supported
to
QMP input lacks member 'execute'
Improvement, except the QMP part is a bit odd.
Let's mention the error message improvments in the commit message.
