Kevin Wolf <kw...@redhat.com> writes:
> Move HMP infrastructure from monitor/misc.c to monitor/hmp.c. This is
> code that can be shared for all targets, so compile it only once.
>
> The amount of function and particularly extern variables in
> monitor_int.h is probably a bit larger than it needs to be, but this way
> no non-trivial code modifications are needed. The interfaces between HMP
> and the monitor core can be cleaned up later.
>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> ---
[...]
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> new file mode 100644
> index 0000000000..3621b195ed
> --- /dev/null
> +++ b/monitor/hmp.c
> @@ -0,0 +1,1415 @@
[...]
> +static int64_t expr_unary(Monitor *mon)
> +{
> + int64_t n;
> + char *p;
> + int ret;
> +
> + switch (*pch) {
> + case '+':
> + next();
> + n = expr_unary(mon);
> + break;
> + case '-':
> + next();
> + n = -expr_unary(mon);
> + break;
> + case '~':
> + next();
> + n = ~expr_unary(mon);
> + break;
> + case '(':
> + next();
> + n = expr_sum(mon);
> + if (*pch != ')') {
> + expr_error(mon, "')' expected");
> + }
> + next();
> + break;
> + case '\'':
> + pch++;
> + if (*pch == '\0') {
> + expr_error(mon, "character constant expected");
> + }
> + n = *pch;
> + pch++;
> + if (*pch != '\'') {
> + expr_error(mon, "missing terminating \' character");
> + }
> + next();
> + break;
> + case '$':
> + {
> + char buf[128], *q;
> + int64_t reg = 0;
> +
> + pch++;
> + q = buf;
> + while ((*pch >= 'a' && *pch <= 'z') ||
> + (*pch >= 'A' && *pch <= 'Z') ||
> + (*pch >= '0' && *pch <= '9') ||
> + *pch == '_' || *pch == '.') {
> + if ((q - buf) < sizeof(buf) - 1) {
> + *q++ = *pch;
> + }
> + pch++;
> + }
> + while (qemu_isspace(*pch)) {
> + pch++;
> + }
> + *q = 0;
> + ret = get_monitor_def(®, buf);
> + if (ret < 0) {
> + expr_error(mon, "unknown register");
> + }
> + n = reg;
> + }
> + break;
> + case '\0':
> + expr_error(mon, "unexpected end of expression");
> + n = 0;
> + break;
> + default:
> + errno = 0;
> + n = strtoull(pch, &p, 0);
checkpatch.pl gripes:
ERROR: consider using qemu_strtoull in preference to strtoull
Let's add a TODO comment.
> + if (errno == ERANGE) {
> + expr_error(mon, "number too large");
> + }
> + if (pch == p) {
> + expr_error(mon, "invalid char '%c' in expression", *p);
> + }
> + pch = p;
> + while (qemu_isspace(*pch)) {
> + pch++;
> + }
> + break;
> + }
> + return n;
> +}
[...]
> +static void monitor_find_completion(void *opaque,
> + const char *cmdline)
> +{
> + MonitorHMP *mon = opaque;
> + char *args[MAX_ARGS];
> + int nb_args, len;
> +
> + /* 1. parse the cmdline */
> + if (parse_cmdline(cmdline, &nb_args, args) < 0) {
> + return;
> + }
> +
> + /* if the line ends with a space, it means we want to complete the
> + * next arg */
checkpatch.pl again:
WARNING: Block comments use a leading /* on a separate line
WARNING: Block comments use a trailing */ on a separate line
Can touch up in my tree.
> + len = strlen(cmdline);
> + if (len > 0 && qemu_isspace(cmdline[len - 1])) {
> + if (nb_args >= MAX_ARGS) {
> + goto cleanup;
> + }
> + args[nb_args++] = g_strdup("");
> + }
> +
> + /* 2. auto complete according to args */
> + monitor_find_completion_by_table(mon, hmp_cmds, args, nb_args);
> +
> +cleanup:
> + free_cmdline_args(args, nb_args);
> +}
[...]
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 368b8297d4..c8289959c0 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
[...]
> @@ -612,245 +580,27 @@ out:
> return output;
> }
>
> -static int compare_cmd(const char *name, const char *list)
> +/**
> + * Is @name in the '|' separated list of names @list?
> + */
> +int hmp_compare_cmd(const char *name, const char *list)
> {
> const char *p, *pstart;
> int len;
> len = strlen(name);
> p = list;
> - for(;;) {
> + for (;;) {
> pstart = p;
> p = qemu_strchrnul(p, '|');
> - if ((p - pstart) == len && !memcmp(pstart, name, len))
> + if ((p - pstart) == len && !memcmp(pstart, name, len)) {
> return 1;
The diff gets confusing here. The function remains unchanged. Good.
> - if (*p == '\0')
> - break;
> - p++;
> - }
> - return 0;
> -}
> -
> -static int get_str(char *buf, int buf_size, const char **pp)
> -{
> - const char *p;
> - char *q;
> - int c;
> -
> - q = buf;
> - p = *pp;
> - while (qemu_isspace(*p)) {
> - p++;
> - }
> - if (*p == '\0') {
> - fail:
> - *q = '\0';
> - *pp = p;
> - return -1;
> - }
> - if (*p == '\"') {
> - p++;
> - while (*p != '\0' && *p != '\"') {
> - if (*p == '\\') {
> - p++;
> - c = *p++;
> - switch (c) {
> - case 'n':
> - c = '\n';
> - break;
> - case 'r':
> - c = '\r';
> - break;
> - case '\\':
> - case '\'':
> - case '\"':
> - break;
> - default:
> - printf("unsupported escape code: '\\%c'\n", c);
> - goto fail;
> - }
> - if ((q - buf) < buf_size - 1) {
> - *q++ = c;
> - }
> - } else {
> - if ((q - buf) < buf_size - 1) {
> - *q++ = *p;
> - }
> - p++;
> - }
> - }
> - if (*p != '\"') {
> - printf("unterminated string\n");
> - goto fail;
> - }
> - p++;
> - } else {
> - while (*p != '\0' && !qemu_isspace(*p)) {
> - if ((q - buf) < buf_size - 1) {
> - *q++ = *p;
> - }
> - p++;
> - }
> - }
> - *q = '\0';
> - *pp = p;
> - return 0;
> -}
> -
> -#define MAX_ARGS 16
> -
> -static void free_cmdline_args(char **args, int nb_args)
> -{
> - int i;
> -
> - assert(nb_args <= MAX_ARGS);
> -
> - for (i = 0; i < nb_args; i++) {
> - g_free(args[i]);
> - }
> -
> -}
> -
> -/*
> - * Parse the command line to get valid args.
> - * @cmdline: command line to be parsed.
> - * @pnb_args: location to store the number of args, must NOT be NULL.
> - * @args: location to store the args, which should be freed by caller, must
> - * NOT be NULL.
> - *
> - * Returns 0 on success, negative on failure.
> - *
> - * NOTE: this parser is an approximate form of the real command parser.
> Number
> - * of args have a limit of MAX_ARGS. If cmdline contains more, it will
> - * return with failure.
> - */
> -static int parse_cmdline(const char *cmdline,
> - int *pnb_args, char **args)
> -{
> - const char *p;
> - int nb_args, ret;
> - char buf[1024];
> -
> - p = cmdline;
> - nb_args = 0;
> - for (;;) {
> - while (qemu_isspace(*p)) {
> - p++;
> }
> if (*p == '\0') {
> break;
> }
> - if (nb_args >= MAX_ARGS) {
> - goto fail;
> - }
> - ret = get_str(buf, sizeof(buf), &p);
> - if (ret < 0) {
> - goto fail;
> - }
> - args[nb_args] = g_strdup(buf);
> - nb_args++;
> + p++;
> }
> - *pnb_args = nb_args;
> return 0;
> -
> - fail:
> - free_cmdline_args(args, nb_args);
> - return -1;
> -}
> -
> -/*
> - * Can command @cmd be executed in preconfig state?
> - */
> -static bool cmd_can_preconfig(const HMPCommand *cmd)
> -{
> - if (!cmd->flags) {
> - return false;
> - }
> -
> - return strchr(cmd->flags, 'p');
> -}
> -
> -static void help_cmd_dump_one(Monitor *mon,
> - const HMPCommand *cmd,
> - char **prefix_args,
> - int prefix_args_nb)
> -{
> - int i;
> -
> - if (runstate_check(RUN_STATE_PRECONFIG) && !cmd_can_preconfig(cmd)) {
> - return;
> - }
> -
> - for (i = 0; i < prefix_args_nb; i++) {
> - monitor_printf(mon, "%s ", prefix_args[i]);
> - }
> - monitor_printf(mon, "%s %s -- %s\n", cmd->name, cmd->params, cmd->help);
> -}
> -
> -/* @args[@arg_index] is the valid command need to find in @cmds */
> -static void help_cmd_dump(Monitor *mon, const HMPCommand *cmds,
> - char **args, int nb_args, int arg_index)
> -{
> - const HMPCommand *cmd;
> - size_t i;
> -
> - /* No valid arg need to compare with, dump all in *cmds */
> - if (arg_index >= nb_args) {
> - for (cmd = cmds; cmd->name != NULL; cmd++) {
> - help_cmd_dump_one(mon, cmd, args, arg_index);
> - }
> - return;
> - }
> -
> - /* Find one entry to dump */
> - for (cmd = cmds; cmd->name != NULL; cmd++) {
> - if (compare_cmd(args[arg_index], cmd->name) &&
> - ((!runstate_check(RUN_STATE_PRECONFIG) ||
> - cmd_can_preconfig(cmd)))) {
> - if (cmd->sub_table) {
> - /* continue with next arg */
> - help_cmd_dump(mon, cmd->sub_table,
> - args, nb_args, arg_index + 1);
> - } else {
> - help_cmd_dump_one(mon, cmd, args, arg_index);
> - }
> - return;
> - }
> - }
> -
> - /* Command not found */
> - monitor_printf(mon, "unknown command: '");
> - for (i = 0; i <= arg_index; i++) {
> - monitor_printf(mon, "%s%s", args[i], i == arg_index ? "'\n" : " ");
> - }
> -}
> -
> -static void help_cmd(Monitor *mon, const char *name)
> -{
> - char *args[MAX_ARGS];
> - int nb_args = 0;
> -
> - /* 1. parse user input */
> - if (name) {
> - /* special case for log, directly dump and return */
> - if (!strcmp(name, "log")) {
> - const QEMULogItem *item;
> - monitor_printf(mon, "Log items (comma separated):\n");
> - monitor_printf(mon, "%-10s %s\n", "none", "remove all logs");
> - for (item = qemu_log_items; item->mask != 0; item++) {
> - monitor_printf(mon, "%-10s %s\n", item->name, item->help);
> - }
> - return;
> - }
> -
> - if (parse_cmdline(name, &nb_args, args) < 0) {
> - return;
> - }
> - }
> -
> - /* 2. dump the contents according to parsed args */
> - help_cmd_dump(mon, hmp_cmds, args, nb_args, 0);
> -
> - free_cmdline_args(args, nb_args);
> }
[...]
Reviewed-by: Markus Armbruster <arm...@redhat.com>
