Re: [Qemu-devel] [PATCH V8 00/13] monitor: support sub command group in auto completion and help

[Wenchao Xia]

于 2013-7-31 0:03, Luiz Capitulino 写道:
> On Fri, 26 Jul 2013 11:20:29 +0800
> Wenchao Xia <address@hidden> wrote:
>
> > This series make auto completion and help functions works normal for sub
> > command, by using reentrant functions. In order to do that, global variables
> > are not directly used in those functions any more. With this series, cmd_table
> > is a member of structure Monitor so it is possible to create a monitor with
> > different command table now, auto completion will work in that monitor. In
> > short, "info" is not treated as a special case now, this series ensure help
> > and auto complete function works normal for any sub command added in the future.
> >
> > Patch 5 replaced cur_mon with rs->mon, it is safe because:
> > monitor_init() calls readline_init() which initialize mon->rs, result is
> > mon->rs->mon == mon. Then qemu_chr_add_handlers() is called, which make
> > monitor_read() function take *mon as its opaque. Later, when user input,
> > monitor_read() is called, where cur_mon is set to *mon by "cur_mon = opaque".
> > If qemu's monitors run in one thread, then later in readline_handle_byte()
> > and readline_comletion(), cur_mon is actually equal to rs->mon, in another
> > word, it points to the monitor instance, so it is safe to replace *cur_mon
> > in those functions.
>
> I've applied this to qmp-next with the change I suggested for
> patch 09/13.
  Thanks a lot!


> > Thanks for Luiz and Eric for reviewing.
> >
> > V2:
> >    General:
> >    To discard *info_comds more graceful, help related function is modified to 
> > support
> > sub command too.
> >    Patch 6/7 are added to improve help related functions.
> >    Patch 5: not directly return to make sure args are freed.
> >
> >    Address Luiz's comments:
> >    Split patch into small series.
> >    struct mon_cmd_t was not moved into header file, instead mon_cmnd_t 
> > *cmd_table is
> > added as a member in struct Monitor.
> >    5/7: drop original code comments for "info" in monitor_find_completion().
> >
> > v3:
> >    5/7: add parameter **args_cmdline in parse_cmdline() to tell next valid
> > parameter's position. This fix the issue in case command length in input is not
> > equal to its name's length such as "help|?", and the case input start with
> > space such as "  s".
> >    7/7: better commit message.
> >
> > v4:
> >    Address Eric's comments:
> >    1/7, 2/7, 4/7: better commit title and message.
> >    1/7 remove useless (char *) in old code, add space around "for ()" in old 
> > code.
> >    3/7: separate code moving patch before usage.
> >    4/7: add space around "for ()" in old code, add min(nb_args, MAX_ARGS) in 
> > free
> > to make code stronger.
> >
> > v5:
> >    4/7: use "a < b ? a : b" instead of macro min.
> >
> > v6:
> >    Address Luiz's comments:
> >    1/13 ~ 5/13: splitted small patches.
> >    5/13: added commit message about the correctness of replacing of cur_mon and
> > test result.
> >    6/13: better comments in code.
> >    7/13: added commit message about the reason of code moving.
> >    8/13: new patch to improve parse_cmdline(), since it is a more generic
> > function now.
> >    9/13: reworked the commit message, better commentes in code, use
> > free_cmdline_args() in clean. It is a bit hard to split this patch into
> > smaller meaning ful ones, so kepted this patch as a relative larger one,
> > with better commit message.
> >    12/13: put case 'S' with case 's' in monitor_find_completion_by_table().
> > moved this patch ahead of patch 13/13.
> >    13/13: this patch is moved behind patch 12/13.
> >
> >    Generic change:
> >    10/13: splitted patch which moved out the reentrant part into a separate
> > function, make review easier. This also avoided re-parsing the command line
> > which does in previous version.
> >    11/13: splitted patch, which simply remove usage of info_cmds and support
> > sub command by re-enter the function.
> >
> > v7:
> >    Address Luiz's comments:
> >    5/13: moved the comments why the change is safe, to cover-letter.
> >    8/13: use assert in free_cmdline_args(), fail when args in input exceed
> > the limit in parse_cmdline().
> >
> > v8:
> >    Address Eric's comments:
> >    Fix typo in commit messages.
> >
> > Wenchao Xia (13):
> >    1 monitor: avoid use of global *cur_mon in cmd_completion()
> >    2 monitor: avoid use of global *cur_mon in file_completion()
> >    3 monitor: avoid use of global *cur_mon in block_completion_it()
> >    4 monitor: avoid use of global *cur_mon in monitor_find_completion()
> >    5 monitor: avoid use of global *cur_mon in readline_completion()
> >    6 monitor: avoid direct use of global variable *mon_cmds
> >    7 monitor: code move for parse_cmdline()
> >    8 monitor: refine parse_cmdline()
> >    9 monitor: support sub command in help
> >    10 monitor: refine monitor_find_completion()
> >    11 monitor: support sub command in auto completion
> >    12 monitor: allow "help" show message for single command in sub group
> >    13 monitor: improve auto complete of "help" for single command in sub group
> >
> >   hmp-commands.hx            |    2 +-
> >   include/monitor/readline.h |    3 +-
> >   monitor.c                  |  438 ++++++++++++++++++++++++++++----------------
> >   readline.c                 |    5 +-
> >   4 files changed, 289 insertions(+), 159 deletions(-)


--
Best Regards

Wenchao Xia