Re: [Qemu-devel] [PATCH V5 7/7] monitor: improve "help" to allow show details of single command in sub group

[Luiz Capitulino <lcapitulino@redhat.com>]

On Sat, 29 Jun 2013 11:53:01 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> A new parameter type 'S' is introduced to allow user input any string.
> "help info block" do not tip extra parameter error now.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  hmp-commands.hx |    2 +-
>  monitor.c       |   30 +++++++++++++++++++++++++++++-
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 915b0d1..8cf5f29 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -11,7 +11,7 @@ ETEXI
>  
>      {
>          .name       = "help|?",
> -        .args_type  = "name:s?",
> +        .args_type  = "name:S?",
>          .params     = "[cmd]",
>          .help       = "show the help",
>          .mhandler.cmd = do_help_cmd,
> diff --git a/monitor.c b/monitor.c
> index 3ef18ee..ebdc2a3 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -83,6 +83,7 @@
>   * 'F'          filename
>   * 'B'          block device name
>   * 's'          string (accept optional quote)
> + * 'S'          supported types, can be any string (accept optional quote)

I think you want to say it just appends the rest of the string.

>   * 'O'          option string of the form NAME=VALUE,...
>   *              parsed according to QemuOptsList given by its name
>   *              Example: 'device:O' uses qemu_device_opts.
> @@ -4011,6 +4012,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;
> +            }
> +            break;
>          default:
>          bad_type:
>              monitor_printf(mon, "%s: unknown type '%c'\n", cmdname, c);
> @@ -4282,7 +4308,9 @@ static void monitor_find_completion_by_table(Monitor *mon,
>                  for (i = 0; i < Q_KEY_CODE_MAX; i++) {
>                      cmd_completion(mon, str, QKeyCode_lookup[i]);
>                  }
> -            } else if (!strcmp(cmd->name, "help|?")) {
> +            }
> +        case 'S':
> +            if (!strcmp(cmd->name, "help|?")) {
>                  monitor_find_completion_by_table(mon, cmd_table,
>                                                   args_cmdline[1]);

This gives a very specific meaning to the S type, doesn't it? Why can't
you treat it just like 's'? Meaning that you could have:

  case 's':
  case 'S':

>              }