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.
Suggested-by: Markus Armbruster <arm...@redhat.com> Signed-off-by: Bandan Das <b...@redhat.com> --- monitor.c | 84 ++++++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 51 insertions(+), 33 deletions(-) diff --git a/monitor.c b/monitor.c index b2561e1..f0c1a8c 100644 --- a/monitor.c +++ b/monitor.c @@ -3685,8 +3685,7 @@ 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 @cmdline is invalid, report to @mon and return NULL. * 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 @@ -3696,23 +3695,19 @@ static const mon_cmd_t *qmp_find_cmd(const char *cmdname) */ static const mon_cmd_t *monitor_parse_command(Monitor *mon, const char *cmdline, - int start, - mon_cmd_t *table, - QDict *qdict) + int *start, + 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='%d'\n", cmdline, *start); #endif /* extract the command name */ - p = get_command_name(cmdline + start, cmdname, sizeof(cmdname)); + p = get_command_name(cmdline + *start, cmdname, sizeof(cmdname)); if (!p) return NULL; @@ -3727,16 +3722,35 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, while (qemu_isspace(*p)) { p++; } + + *start = p - cmdline; /* 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, cmdline, start, + 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 @qdict, and return it. + */ + +static QDict *monitor_parse_arguments(Monitor *mon, + const char *cmdline, + int *start, + const mon_cmd_t *cmd) +{ + const char *typestr; + char *key; + int c; + const char *p = cmdline + *start; + char buf[1024]; + QDict *qdict = qdict_new(); + /* parse the parameters */ typestr = cmd->args_type; for(;;) { @@ -3766,14 +3780,14 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, switch(c) { case 'F': monitor_printf(mon, "%s: filename expected\n", - cmdname); + cmd->name); break; case 'B': monitor_printf(mon, "%s: block device name expected\n", - cmdname); + cmd->name); break; default: - monitor_printf(mon, "%s: string expected\n", cmdname); + monitor_printf(mon, "%s: string expected\n", cmd->name); break; } goto fail; @@ -3915,7 +3929,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, goto fail; /* Check if 'i' is greater than 32-bit */ if ((c == 'i') && ((val >> 32) & 0xffffffff)) { - monitor_printf(mon, "\'%s\' has failed: ", cmdname); + monitor_printf(mon, "\'%s\' has failed: ", cmd->name); monitor_printf(mon, "integer is for 32-bit values\n"); goto fail; } else if (c == 'M') { @@ -4023,7 +4037,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, if(!is_valid_option(p, typestr)) { monitor_printf(mon, "%s: unsupported option -%c\n", - cmdname, *p); + cmd->name, *p); goto fail; } else { skip_key = 1; @@ -4057,7 +4071,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, len = strlen(p); if (len <= 0) { monitor_printf(mon, "%s: string expected\n", - cmdname); + cmd->name); break; } qdict_put(qdict, key, qstring_from_str(p)); @@ -4066,7 +4080,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, break; default: bad_type: - monitor_printf(mon, "%s: unknown type '%c'\n", cmdname, c); + monitor_printf(mon, "%s: unknown type '%c'\n", cmd->name, c); goto fail; } g_free(key); @@ -4077,13 +4091,14 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, p++; if (*p != '\0') { monitor_printf(mon, "%s: extraneous characters at the end of line\n", - cmdname); + cmd->name); goto fail; } - return cmd; + return qdict; fail: + QDECREF(qdict); g_free(key); return NULL; } @@ -4114,12 +4129,17 @@ static void handle_user_command(Monitor *mon, const char *cmdline) { QDict *qdict; const mon_cmd_t *cmd; + int start = 0; - qdict = qdict_new(); + cmd = monitor_parse_command(mon, cmdline, &start, mon->cmd_table); + if (!cmd) { + return; + } - cmd = monitor_parse_command(mon, cmdline, 0, mon->cmd_table, qdict); - if (!cmd) - goto out; + qdict = monitor_parse_arguments(mon, cmdline, &start, cmd); + if (!qdict) { + return; + } if (handler_is_async(cmd)) { user_async_cmd_handler(mon, cmd, qdict); @@ -4137,8 +4157,6 @@ static void handle_user_command(Monitor *mon, const char *cmdline) cmd->mhandler.cmd(mon, qdict); } -out: - QDECREF(qdict); } static void cmd_completion(Monitor *mon, const char *name, const char *list) -- 2.1.0