Re: [PATCH v3 23/34] qapi: Simplify how qmp_dispatch() gets the request ID

[Marc-André Lureau]

On Sun, Mar 15, 2020 at 3:51 PM Markus Armbruster <address@hidden> wrote:
>
> We convert the request object to a QDict twice: first in
> qmp_dispatch() to get the request ID, and then again in
> qmp_dispatch_check_obj(), which converts to QDict, then checks and
> returns it.  We can't get the request ID from the latter, because it's
> null when the qdict flunks the checks.
>
> Move getting the request ID into qmp_dispatch_check_obj().
>

I don't see this is a an improvement. qmp_dispatch_check_obj() doesn't
care about id.

And it doesn't look like it is saving cycles either.

Is that worth it?


Code change is ok otherwise,

> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  qapi/qmp-dispatch.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 550d1fe8d2..112d29a9ab 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -20,7 +20,7 @@
>  #include "qapi/qmp/qbool.h"
>
>  static QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob,
> -                                     Error **errp)
> +                                     QObject **id, Error **errp)
>  {
>      const char *exec_key = NULL;
>      const QDictEntry *ent;
> @@ -30,10 +30,13 @@ static QDict *qmp_dispatch_check_obj(const QObject 
> *request, bool allow_oob,
>
>      dict = qobject_to(QDict, request);
>      if (!dict) {
> +        *id = NULL;
>          error_setg(errp, "QMP input must be a JSON object");
>          return NULL;
>      }
>
> +    *id = qdict_get(dict, "id");
> +
>      for (ent = qdict_first(dict); ent;
>           ent = qdict_next(dict, ent)) {
>          arg_name = qdict_entry_key(ent);
> @@ -103,12 +106,12 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject 
> *request,
>      const char *command;
>      QDict *args;
>      QmpCommand *cmd;
> -    QDict *dict = qobject_to(QDict, request);
> -    QObject *id = dict ? qdict_get(dict, "id") : NULL;
> +    QDict *dict;
> +    QObject *id;
>      QObject *ret = NULL;
>      QDict *rsp = NULL;
>
> -    dict = qmp_dispatch_check_obj(request, allow_oob, &err);
> +    dict = qmp_dispatch_check_obj(request, allow_oob, &id, &err);
>      if (!dict) {
>          goto out;
>      }
> --
> 2.21.1
>
>


-- 
Marc-André Lureau