| Message ID | 1356601227-8707-3-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 27 Dec 2012 17:40:25 +0800 Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote: > This patch make parsing of hmp command aware of that it may > have sub command. Also discard simple encapsulation function > monitor_find_command() and qmp_find_cmd(). > > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > --- > monitor.c | 44 +++++++++++++++++++++++++++----------------- > 1 files changed, 27 insertions(+), 17 deletions(-) > > diff --git a/monitor.c b/monitor.c > index c7b3014..e4d4c59 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -130,6 +130,7 @@ typedef struct mon_cmd_t { > MonitorCompletion *cb, void *opaque); > } mhandler; > int flags; > + struct mon_cmd_t *sub_table; > } mon_cmd_t; > > /* file descriptors passed via SCM_RIGHTS */ > @@ -3534,21 +3535,17 @@ 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) > -{ > - 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); > -} Touching qmp_find_cmd() is unrelated to this series. > - > -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. > { > - const char *p, *typestr; > + const char *p, *p1, *typestr; > int c; > const mon_cmd_t *cmd; > char cmdname[256]; > @@ -3564,12 +3561,25 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, > if (!p) > return NULL; > > - cmd = monitor_find_command(cmdname); > + cmd = search_dispatch_table(table, cmdname); > if (!cmd) { > monitor_printf(mon, "unknown command: '%s'\n", cmdname); > return NULL; > } > > + /* search sub command */ > + if (cmd->sub_table != NULL) { > + p1 = p; > + /* check if user set additional command */ > + while (qemu_isspace(*p1)) { > + p1++; > + } > + if (*p1 == '\0') { > + return cmd; > + } > + 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. 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. > + } > + > /* parse the parameters */ > typestr = cmd->args_type; > for(;;) { > @@ -3925,7 +3935,7 @@ static void handle_user_command(Monitor *mon, const char *cmdline) > > qdict = qdict_new(); > > - cmd = monitor_parse_command(mon, cmdline, qdict); > + cmd = hmp_parse_command(mon, cmdline, mon_cmds, qdict); > if (!cmd) > goto out; > > @@ -4483,7 +4493,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) > goto err_out; > } > > - cmd = qmp_find_cmd(cmd_name); > + cmd = search_dispatch_table(qmp_cmds, cmd_name); > if (!cmd) { > qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name); > goto err_out;
>> -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. > 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. > >> + } >> + >> /* parse the parameters */ >> typestr = cmd->args_type; >> for(;;) { >> @@ -3925,7 +3935,7 @@ static void handle_user_command(Monitor *mon, const char *cmdline) >> >> qdict = qdict_new(); >> >> - cmd = monitor_parse_command(mon, cmdline, qdict); >> + cmd = hmp_parse_command(mon, cmdline, mon_cmds, qdict); >> if (!cmd) >> goto out; >> >> @@ -4483,7 +4493,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) >> goto err_out; >> } >> >> - cmd = qmp_find_cmd(cmd_name); >> + cmd = search_dispatch_table(qmp_cmds, cmd_name); >> if (!cmd) { >> qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name); >> goto err_out; > >
于 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. >>
wenchao xia <xiaxia347os@163.com> writes: > 于 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 agree with the change, but it needs to be mentioned in the commit message. Separate patch would be even 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. I'll review v4.
diff --git a/monitor.c b/monitor.c index c7b3014..e4d4c59 100644 --- a/monitor.c +++ b/monitor.c @@ -130,6 +130,7 @@ typedef struct mon_cmd_t { MonitorCompletion *cb, void *opaque); } mhandler; int flags; + struct mon_cmd_t *sub_table; } mon_cmd_t; /* file descriptors passed via SCM_RIGHTS */ @@ -3534,21 +3535,17 @@ 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) -{ - 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); -} - -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) { - const char *p, *typestr; + const char *p, *p1, *typestr; int c; const mon_cmd_t *cmd; char cmdname[256]; @@ -3564,12 +3561,25 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, if (!p) return NULL; - cmd = monitor_find_command(cmdname); + cmd = search_dispatch_table(table, cmdname); if (!cmd) { monitor_printf(mon, "unknown command: '%s'\n", cmdname); return NULL; } + /* search sub command */ + if (cmd->sub_table != NULL) { + p1 = p; + /* check if user set additional command */ + while (qemu_isspace(*p1)) { + p1++; + } + if (*p1 == '\0') { + return cmd; + } + return hmp_parse_command(mon, p, cmd->sub_table, qdict); + } + /* parse the parameters */ typestr = cmd->args_type; for(;;) { @@ -3925,7 +3935,7 @@ static void handle_user_command(Monitor *mon, const char *cmdline) qdict = qdict_new(); - cmd = monitor_parse_command(mon, cmdline, qdict); + cmd = hmp_parse_command(mon, cmdline, mon_cmds, qdict); if (!cmd) goto out; @@ -4483,7 +4493,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) goto err_out; } - cmd = qmp_find_cmd(cmd_name); + cmd = search_dispatch_table(qmp_cmds, cmd_name); if (!cmd) { qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name); goto err_out;
This patch make parsing of hmp command aware of that it may have sub command. Also discard simple encapsulation function monitor_find_command() and qmp_find_cmd(). Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> --- monitor.c | 44 +++++++++++++++++++++++++++----------------- 1 files changed, 27 insertions(+), 17 deletions(-)