From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34981) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V4LzG-0004QM-In for qemu-devel@nongnu.org; Tue, 30 Jul 2013 22:18:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V4Lz7-0003pl-Fl for qemu-devel@nongnu.org; Tue, 30 Jul 2013 22:18:02 -0400 Received: from e28smtp08.in.ibm.com ([122.248.162.8]:55508) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V4Lz6-0003pb-UW for qemu-devel@nongnu.org; Tue, 30 Jul 2013 22:17:53 -0400 Received: from /spool/local by e28smtp08.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 31 Jul 2013 07:37:35 +0530 Received: from d28relay04.in.ibm.com (d28relay04.in.ibm.com [9.184.220.61]) by d28dlp01.in.ibm.com (Postfix) with ESMTP id CAE2AE004F for ; Wed, 31 Jul 2013 07:47:49 +0530 (IST) Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay04.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r6V2HZtA40763568 for ; Wed, 31 Jul 2013 07:47:38 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r6V2Hccx021374 for ; Wed, 31 Jul 2013 12:17:38 +1000 Message-ID: <51F873B8.4070807@linux.vnet.ibm.com> Date: Wed, 31 Jul 2013 10:17:28 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1374808842-11051-1-git-send-email-xiawenc@linux.vnet.ibm.com> <20130730120311.7f1e44a7@redhat.com> In-Reply-To: <20130730120311.7f1e44a7@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V8 00/13] monitor: support sub command group in auto completion and help List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com 于 2013-7-31 0:03, Luiz Capitulino 写道: > On Fri, 26 Jul 2013 11:20:29 +0800 > Wenchao Xia 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