[Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help

[Wenchao Xia]

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.

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.

V9:
  General:
  6: new patch, I found that call can be optimized, so move it.
  Address Luiz's comments:
  7: split function to init the monitor.
  8: move the cmd_table init code to parse_cmdlinethe splitted function.
  11: directly return in fail calling parse_cmdline(), in help_cmd_dump().
  Other:
  After discuss with Luiz, it would be good to add test case for qmp fd-add
related stuff. Current test case is 045, which doesn't cover the case
that fd is added by SCM at runtime, and that case is related to monitor.
I have drafted a version, which can pass both on upstream and this series,
but it may require a bit more code, so send it later as another series.

Wenchao Xia (15):
  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: call sortcmdlist() only one time
  7 monitor: split off monitor_data_init()
  8 monitor: avoid direct use of global variable *mon_cmds
  9 monitor: code move for parse_cmdline()
  10 monitor: refine parse_cmdline()
  11 monitor: support sub command in help
  12 monitor: refine monitor_find_completion()
  13 monitor: support sub command in auto completion
  14 monitor: allow "help" show message for single command in sub group
  15 monitor: improve auto complete of "help" for single command in sub group

 hmp-commands.hx            |    2 +-
 include/monitor/readline.h |    3 +-
 monitor.c                  |  460 ++++++++++++++++++++++++++++---------------
 readline.c                 |    5 +-
 4 files changed, 304 insertions(+), 166 deletions(-)