Re: [Qemu-devel] [PATCH V5 4/6] HMP: filter out space before check of sub-command

[Markus Armbruster]

Wenchao Xia <address@hidden> writes:

> 于 2013-1-12 4:12, Luiz Capitulino 写道:
>> On Fri, 11 Jan 2013 17:14:03 +0800
>> Wenchao Xia <address@hidden> 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 <address@hidden>
>>> ---
>>>   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.

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.

[...]