On Wed, 19 Dec 2012 18:17:09 +0800
Wenchao Xia <xiaw...@linux.vnet.ibm.com> wrote:
> This patch enable sub info command handler getting meaningful
> parameter.
>
> Signed-off-by: Wenchao Xia <xiaw...@linux.vnet.ibm.com>
> ---
> hmp-commands.hx | 2 +-
> monitor.c | 79
> +++++++++++++++++++++++++++++++++++++++----------------
> 2 files changed, 57 insertions(+), 24 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 010b8c9..667fab8 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1486,7 +1486,7 @@ ETEXI
>
> {
> .name = "info",
> - .args_type = "item:s?",
> + .args_type = "item:S?",
> .params = "[subcommand]",
> .help = "show various information about the system state",
> .mhandler.cmd = do_info,
> diff --git a/monitor.c b/monitor.c
> index 797680f..ce0e74d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -464,6 +464,11 @@ QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) !=
> QEVENT_MAX)
> MonitorEventState monitor_event_state[QEVENT_MAX];
> QemuMutex monitor_event_state_lock;
>
> +static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> + const char *cmdline,
> + const mon_cmd_t *table,
> + QDict *qdict);
Please, move the function instead.
> +
> /*
> * Emits the event to every monitor instance
> */
> @@ -809,26 +814,29 @@ static void user_async_cmd_handler(Monitor *mon, const
> mon_cmd_t *cmd,
> static void do_info(Monitor *mon, const QDict *qdict)
> {
> const mon_cmd_t *cmd;
> + QDict *qdict_info;
> const char *item = qdict_get_try_str(qdict, "item");
>
> if (!item) {
> goto help;
> }
>
> - for (cmd = info_cmds; cmd->name != NULL; cmd++) {
> - if (compare_cmd(item, cmd->name))
> - break;
> - }
> + qdict_info = qdict_new();
>
> - if (cmd->name == NULL) {
> - goto help;
> + cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
> + if (!cmd) {
> + QDECREF(qdict_info);
> + /* don't help here, to avoid error message got ignored */
> + return;
> }
>
> - cmd->mhandler.info(mon, NULL);
> + cmd->mhandler.info(mon, qdict_info);
> + QDECREF(qdict_info);
> return;
>
> help:
> help_cmd(mon, "info");
> + return;
> }
>
> CommandInfoList *qmp_query_commands(Error **errp)
> @@ -3534,18 +3542,15 @@ static const mon_cmd_t *search_dispatch_table(const
> mon_cmd_t *disp_table,
> return NULL;
> }
>
> -static const mon_cmd_t *monitor_find_command(const char *cmdname)
> +static const mon_cmd_t *monitor_find_command(const char *cmdname,
> + const mon_cmd_t *table)
> {
> - return search_dispatch_table(mon_cmds, cmdname);
> -}
> -
> -static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
> -{
> - return search_dispatch_table(qmp_cmds, cmdname);
> + return search_dispatch_table(table, cmdname);
Please, keep only search_dispatch_table().
Actually, maybe you could change handle_qmp_command() to just loop through
command names and compare them with memcpy() (in a different patch, please)
then you keep monitor_find_command() for handle_hmp_command().
> }
>
> static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> const char *cmdline,
> + const mon_cmd_t *table,
> QDict *qdict)
> {
> const char *p, *typestr;
> @@ -3564,7 +3569,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor
> *mon,
> if (!p)
> return NULL;
>
> - cmd = monitor_find_command(cmdname);
> + cmd = monitor_find_command(cmdname, table);
> if (!cmd) {
> monitor_printf(mon, "unknown command: '%s'\n", cmdname);
> return NULL;
> @@ -3872,6 +3877,31 @@ static const mon_cmd_t *monitor_parse_command(Monitor
> *mon,
> }
> }
> break;
> + case 'S':
> + {
> + /* package all remaining string */
> + int len;
> +
> + while (qemu_isspace(*p)) {
> + p++;
> + }
> + if (*typestr == '?') {
> + typestr++;
> + if (*p == '\0') {
> + /* no remaining string: NULL argument */
> + break;
> + }
> + }
> + len = strlen(p);
> + if (len <= 0) {
> + monitor_printf(mon, "%s: string expected\n",
> + cmdname);
> + break;
> + }
> + qdict_put(qdict, key, qstring_from_str(p));
> + p += len;
> + }
This is a true HMP-style hack :)
I don't know how to make this better though (at least not without asking you
to re-write the whole thing). Maybe Markus has a better idea?
> + break;
> default:
> bad_type:
> monitor_printf(mon, "%s: unknown type '%c'\n", cmdname, c);
> @@ -3925,7 +3955,7 @@ static void handle_user_command(Monitor *mon, const
> char *cmdline)
>
> qdict = qdict_new();
>
> - cmd = monitor_parse_command(mon, cmdline, qdict);
> + cmd = monitor_parse_command(mon, cmdline, mon_cmds, qdict);
> if (!cmd)
> goto out;
>
> @@ -4144,12 +4174,7 @@ static void monitor_find_completion(const char
> *cmdline)
> break;
> case 's':
> /* XXX: more generic ? */
> - if (!strcmp(cmd->name, "info")) {
> - readline_set_completion_index(cur_mon->rs, strlen(str));
> - for(cmd = info_cmds; cmd->name != NULL; cmd++) {
> - cmd_completion(str, cmd->name);
> - }
> - } else if (!strcmp(cmd->name, "sendkey")) {
> + if (!strcmp(cmd->name, "sendkey")) {
> char *sep = strrchr(str, '-');
> if (sep)
> str = sep + 1;
> @@ -4163,6 +4188,14 @@ static void monitor_find_completion(const char
> *cmdline)
> cmd_completion(str, cmd->name);
> }
> }
> + case 'S':
> + /* generic string packaged */
> + if (!strcmp(cmd->name, "info")) {
> + readline_set_completion_index(cur_mon->rs, strlen(str));
> + for (cmd = info_cmds; cmd->name != NULL; cmd++) {
> + cmd_completion(str, cmd->name);
> + }
> + }
> break;
> default:
> break;
> @@ -4483,7 +4516,7 @@ static void handle_qmp_command(JSONMessageParser
> *parser, QList *tokens)
> goto err_out;
> }
>
> - cmd = qmp_find_cmd(cmd_name);
> + cmd = monitor_find_command(cmd_name, qmp_cmds);
> if (!cmd) {
> qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
> goto err_out;
