Re: [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command

[wenchao xia <xiaxia347os@163.com>]

于 2013-1-10 11:30, Wenchao Xia 写道:
>>> -static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
>>> -{
>>> -    return search_dispatch_table(qmp_cmds, cmdname);
>>> -}
>>
>> Touching qmp_find_cmd() is unrelated to this series.
>>
>    OK.
>
>>> -
>>> -static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>> -                                              const char *cmdline,
>>> -                                              QDict *qdict)
>>> +/*
>>> + * Try find the target mon_cmd_t instance, if it have sub_table and
>>> have string
>>> + * for it, this function will try search it with remain string, and
>>> if not
>>> + * found it return NULL.
>>> + */
>>> +static const mon_cmd_t *hmp_parse_command(Monitor *mon,
>>> +                                          const char *cmdline,
>>> +                                          mon_cmd_t *table,
>>> +                                          QDict *qdict)
>>
>> Renaming is also not necessary, but if you do it please do in a different
>> patch.
>>
>    OK.
>
>>>   {
>>> +        }
>>> +        return hmp_parse_command(mon, p, cmd->sub_table, qdict);
>>
>> This works, and seems less hack then the previous attempt.
>>
>> Except for a small detail. This:
>>
>>    (qemu) info bla
>>
>> Currently returns a list of info commands, but with this series it
>> returns
>> an error.
>>
>    Yep, I think command completion is another topic, but I am pretty
> sure it can be solved in this framework, for that it is not hack and
> what need to do is, let command completion function knows there may be
> another sub layer too.
>
   Sorry I missed your point. Returning an error instead of info
commands, is made on purpose. IMHO always print the commands
make user confused, message tipping what is wrong is better.

>> I'd like to get a reviewed-by from Markus before applying this as,
>> besides
>> this being his idea, I honestly have a very hard time knowing how to
>> move old
>> hmp hackery forward.
>>