[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Wenchao Xia
Subject: Re: [Qemu-devel] [PATCH V8 00/13] monitor: support sub command group in auto completion and help
Date: Wed, 31 Jul 2013 10:17:28 +0800
User-agent: Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130620 Thunderbird/17.0.7

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

   To discard *info_comds more graceful, help related function is modified to 
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().

   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.

   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 
   3/7: separate code moving patch before usage.
   4/7: add space around "for ()" in old code, add min(nb_args, MAX_ARGS) in 
to make code stronger.

   4/7: use "a < b ? a : b" instead of macro min.

   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.

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

   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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]