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

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013-7-31 0:03, Luiz Capitulino 写道:
> On Fri, 26 Jul 2013 11:20:29 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> 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