diff mbox series

[11/12] qga: process_event() simplification

Message ID 20180706121354.2021-12-marcandre.lureau@redhat.com
State New
Headers show
Series RFC: monitor: various code simplification and fixes | expand

Commit Message

Marc-André Lureau July 6, 2018, 12:13 p.m. UTC
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.lureau@redhat.com>
---
 qga/main.c | 58 ++++++++++++++----------------------------------------
 1 file changed, 15 insertions(+), 43 deletions(-)

Comments

Markus Armbruster July 17, 2018, 3:25 p.m. UTC | #1
Copying the maintainer Michael Roth.

Marc-André Lureau <marcandre.lureau@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.lureau@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.
diff mbox series

Patch

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 */