Bandan Das <b...@redhat.com> writes:
> There's too much going on in monitor_parse_command().
> Split up the arguments parsing bits into a separate function
> monitor_parse_arguments(). Let the original function check for
> command validity and sub-commands if any and return data (*cmd)
> that the newly introduced function can process and return a
> QDict. Also, pass a pointer to the cmdline to track current
> parser location.
>
> Suggested-by: Markus Armbruster <arm...@redhat.com>
> Signed-off-by: Bandan Das <b...@redhat.com>
> ---
> monitor.c | 100
> ++++++++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 58 insertions(+), 42 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index b2561e1..a89cbbb 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3683,43 +3683,36 @@ static const mon_cmd_t *qmp_find_cmd(const char
> *cmdname)
> }
>
> /*
> - * Parse @cmdline according to command table @table.
> - * If @cmdline is blank, return NULL.
> - * If it can't be parsed, report to @mon, and return NULL.
> - * Else, insert command arguments into @qdict, and return the command.
> - * If a sub-command table exists, and if @cmdline contains an additional
> string
> - * for a sub-command, this function will try to search the sub-command table.
> - * If no additional string for a sub-command is present, this function will
> - * return the command found in @table.
> - * Do not assume the returned command points into @table! It doesn't
> - * when the command is a sub-command.
> + * Parse command name from @cmdp according to command table @table.
> + * If blank, return NULL.
> + * Else, if no valid command can be found, report to @mon, and return
> + * NULL.
> + * Else, change @cmdp to point right behind the name, and return its
> + * command table entry.
> + * Do not assume the return value points into @table! It doesn't when
> + * the command is found in a sub-command table.
> */
> static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> - const char *cmdline,
> - int start,
> - mon_cmd_t *table,
> - QDict *qdict)
> + const char **cmdp,
> + mon_cmd_t *table)
> {
> - const char *p, *typestr;
> - int c;
> + const char *p;
> const mon_cmd_t *cmd;
> char cmdname[256];
> - char buf[1024];
> - char *key;
>
> #ifdef DEBUG
> - monitor_printf(mon, "command='%s', start='%d'\n", cmdline, start);
> + monitor_printf(mon, "command='%s', start='%c'\n", cmdline, **cmdp);
> #endif
With DEBUG defined:
CC x86_64-softmmu/monitor.o
/work/armbru/qemu/monitor.c: In function ‘monitor_parse_command’:
/work/armbru/qemu/monitor.c:3704:55: error: ‘cmdline’ undeclared (first use in
this function)
monitor_printf(mon, "command='%s', start='%c'\n", cmdline, **cmdp);
Recommend to fix this by reordering patches: 4 3 1 2.
>
> /* extract the command name */
> - p = get_command_name(cmdline + start, cmdname, sizeof(cmdname));
> + p = get_command_name(*cmdp, cmdname, sizeof(cmdname));
> if (!p)
> return NULL;
>
> cmd = search_dispatch_table(table, cmdname);
> if (!cmd) {
> monitor_printf(mon, "unknown command: '%.*s'\n",
> - (int)(p - cmdline), cmdline);
> + (int)(p - *cmdp), *cmdp);
> return NULL;
> }
>
> @@ -3727,16 +3720,34 @@ static const mon_cmd_t *monitor_parse_command(Monitor
> *mon,
> while (qemu_isspace(*p)) {
> p++;
> }
> +
> + *cmdp = p;
> /* search sub command */
> - if (cmd->sub_table != NULL) {
> - /* check if user set additional command */
> - if (*p == '\0') {
> - return cmd;
> - }
> - return monitor_parse_command(mon, cmdline, p - cmdline,
> - cmd->sub_table, qdict);
> + if (cmd->sub_table != NULL && *p != '\0') {
> + return monitor_parse_command(mon, cmdp, cmd->sub_table);
> }
>
> + return cmd;
> +}
> +
> +/*
> + * Parse arguments for @cmd
> + * If it can't be parsed, report to @mon, and return NULL.
> + * Else, insert command arguments into a QDict, and return it.
> + * Note: On success, caller has to free the QDict structure
> + */
Since you'll have to respin anyway: humor me, and end the sentence with
a period.
[...]
