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

[wenchao xia]

于 2013-1-10 11:30, 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.
  Sorry I missed your point. Returning an error instead of info
commands, is made on purpose. IMHO always print the commands
make user confused, message tipping what is wrong is better.

> > 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.