Re: [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command

[Wenchao Xia]

> > -static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
> > -{
> > -    return search_dispatch_table(qmp_cmds, cmdname);
> > -}
>
> Touching qmp_find_cmd() is unrelated to this series.
  OK.

> > -
> > -static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> > -                                              const char *cmdline,
> > -                                              QDict *qdict)
> > +/*
> > + * Try find the target mon_cmd_t instance, if it have sub_table and have string
> > + * for it, this function will try search it with remain string, and if not
> > + * found it return NULL.
> > + */
> > +static const mon_cmd_t *hmp_parse_command(Monitor *mon,
> > +                                          const char *cmdline,
> > +                                          mon_cmd_t *table,
> > +                                          QDict *qdict)
>
> Renaming is also not necessary, but if you do it please do in a different
> patch.
  OK.

> >   {
> > +        }
> > +        return hmp_parse_command(mon, p, cmd->sub_table, qdict);
>
> This works, and seems less hack then the previous attempt.
>
> Except for a small detail. This:
>
>    (qemu) info bla
>
> Currently returns a list of info commands, but with this series it returns
> an error.
  Yep, I think command completion is another topic, but I am pretty
sure it can be solved in this framework, for that it is not hack and
what need to do is, let command completion function knows there may be
another sub layer too.

> I'd like to get a reviewed-by from Markus before applying this as, besides
> this being his idea, I honestly have a very hard time knowing how to move old
> hmp hackery forward.
>
> > +    }
> > +
> >       /* parse the parameters */
> >       typestr = cmd->args_type;
> >       for(;;) {
> > @@ -3925,7 +3935,7 @@ static void handle_user_command(Monitor *mon, const char 
> > *cmdline)
> >
> >       qdict = qdict_new();
> >
> > -    cmd = monitor_parse_command(mon, cmdline, qdict);
> > +    cmd = hmp_parse_command(mon, cmdline, mon_cmds, qdict);
> >       if (!cmd)
> >           goto out;
> >
> > @@ -4483,7 +4493,7 @@ static void handle_qmp_command(JSONMessageParser *parser, 
> > QList *tokens)
> >           goto err_out;
> >       }
> >
> > -    cmd = qmp_find_cmd(cmd_name);
> > +    cmd = search_dispatch_table(qmp_cmds, cmd_name);
> >       if (!cmd) {
> >           qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
> >           goto err_out;


--
Best Regards

Wenchao Xia