From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38668) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tuwre-0002XJ-Kk for qemu-devel@nongnu.org; Mon, 14 Jan 2013 22:07:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tuwrd-00048h-An for qemu-devel@nongnu.org; Mon, 14 Jan 2013 22:07:02 -0500 Received: from e28smtp01.in.ibm.com ([122.248.162.1]:49365) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tuwrc-00047e-NP for qemu-devel@nongnu.org; Mon, 14 Jan 2013 22:07:01 -0500 Received: from /spool/local by e28smtp01.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 15 Jan 2013 08:35:23 +0530 Received: from d28relay03.in.ibm.com (d28relay03.in.ibm.com [9.184.220.60]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id 25C8E125804E for ; Tue, 15 Jan 2013 08:37:11 +0530 (IST) Received: from d28av01.in.ibm.com (d28av01.in.ibm.com [9.184.220.63]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r0F36kLr31654106 for ; Tue, 15 Jan 2013 08:36:50 +0530 Received: from d28av01.in.ibm.com (loopback [127.0.0.1]) by d28av01.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r0F36n44000325 for ; Tue, 15 Jan 2013 03:06:49 GMT Message-ID: <50F4C7B3.7040303@linux.vnet.ibm.com> Date: Tue, 15 Jan 2013 11:06:27 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1357895645-30359-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1357895645-30359-5-git-send-email-xiawenc@linux.vnet.ibm.com> <20130111181209.5f045b19@doriath.home> <50F36940.7050709@linux.vnet.ibm.com> <87zk0bx4g4.fsf@blackfin.pond.sub.org> In-Reply-To: <87zk0bx4g4.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V5 4/6] HMP: filter out space before check of sub-command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, chenwj@iis.sinica.edu.tw, Luiz Capitulino 于 2013-1-14 21:46, Markus Armbruster 写道: > Wenchao Xia writes: > >> 于 2013-1-12 4:12, Luiz Capitulino 写道: >>> On Fri, 11 Jan 2013 17:14:03 +0800 >>> Wenchao Xia wrote: >>> >>>> This fix the case when user input "@command ". Original >>>> it will return NULL for monitor_parse_command(), now >>>> it will return the @command related instance. >>>> >>>> Signed-off-by: Wenchao Xia >>>> --- >>>> monitor.c | 3 +++ >>>> 1 files changed, 3 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/monitor.c b/monitor.c >>>> index 5435dc3..7b752a2 100644 >>>> --- a/monitor.c >>>> +++ b/monitor.c >>>> @@ -3588,6 +3588,9 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, >>>> if (cmd->sub_table != NULL) { >>>> p1 = p; >>>> /* check if user set additional command */ >>>> + while (qemu_isspace(*p1)) { >>>> + p1++; >>>> + } >>> >>> Is there a reason for this to be in a different patch? I mean, why don't >>> you squash this into the previous patch? >>> >> Markus suggest to do it > > Misunderstanding, sorry. My point was we skip over whitespace twice: > first here, and then again in monitor_parse_command(). Quote: > > The check whether non-space characters follow is awkward. We need > it only because we want to handle "@cmdline is blank" differently > than "it can't be parsed", but monitor_parse_command() returns NULL > for both cases. > > Merely an observation, not a request to do anything about it in this > series: > > If we care, we can try to do better in a follow-up patch. > > A possible way to do better is to have a handler for command "", to be > invoked for entirely blank lines. In table info_cmds, the handler would > be do_info_help. Then prefix commands don't need their handler anymore > (it's in the sub_table), and sub_table can go into the mhandler union. > Thanks for the declaration. Have a handler for command "" seems good, but need a careful check about the parsing code since '\0' is a key value. *sub_table plays an extra role of tagging if it have sub-commands so I think it is better to not folder it into mhandler union. > We'd need to suppress the "unknown command" error for cmdname "". > > If this isn't clear, but you want to understand it, I can write a patch > on top. > >> and I think this make things clear. I am OK >> to merge it and drop p1. > > I'm not sure it makes things clearer. It makes behavior of "info" > without argument change twice in the series, though, in the preceding > patch, and in this one. I think I agree with Luiz it's better to squash > the two together again. > > [...] > -- Best Regards Wenchao Xia