Patchwork [V5,7/7] monitor: improve "help" to allow show details of single command in sub group

login
register
mail settings
Submitter Wayne Xia
Date June 29, 2013, 3:53 a.m.
Message ID <1372477981-7512-8-git-send-email-xiawenc@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/255713/
State New
Headers show

Comments

Wayne Xia - June 29, 2013, 3:53 a.m.
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(-)
Luiz Capitulino - July 8, 2013, 4:17 p.m.
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':

>              }
Wayne Xia - July 10, 2013, 6:47 a.m.
于 2013-7-9 0:17, Luiz Capitulino 写道:
> 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':
>
   OK, will fix it.


>>               }
>
>

Patch

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)
  * '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]);
             }