[Qemu-devel] [PATCH V10 00/15] monitor: support sub command group in auto completion and help

[Qemu-devel] [PATCH V10 00/15] monitor: support sub command group in auto completion and help

[Wenchao Xia]

This series make auto completion and help functions works normal for sub
command, by using reentrant functions. In order to do that, global variables
are not directly used in those functions any more. With this series, cmd_table
is a member of structure Monitor so it is possible to create a monitor with
different command table now, auto completion will work in that monitor. In
short, "info" is not treated as a special case now, this series ensure help
and auto complete function works normal for any sub command added in the future.

Patch 5 replaced cur_mon with rs->mon, it is safe because:
monitor_init() calls readline_init() which initialize mon->rs, result is
mon->rs->mon == mon. Then qemu_chr_add_handlers() is called, which make
monitor_read() function take *mon as its opaque. Later, when user input,
monitor_read() is called, where cur_mon is set to *mon by "cur_mon = opaque".
If qemu's monitors run in one thread, then later in readline_handle_byte()
and readline_comletion(), cur_mon is actually equal to rs->mon, in another
word, it points to the monitor instance, so it is safe to replace *cur_mon
in those functions.

Thanks for Luiz and Eric for reviewing.

V2:
  General:
  To discard *info_comds more graceful, help related function is modified to support
sub command too.
  Patch 6/7 are added to improve help related functions.
  Patch 5: not directly return to make sure args are freed.

  Address Luiz's comments:
  Split patch into small series.
  struct mon_cmd_t was not moved into header file, instead mon_cmnd_t *cmd_table is
added as a member in struct Monitor.
  5/7: drop original code comments for "info" in monitor_find_completion().

v3:
  5/7: add parameter **args_cmdline in parse_cmdline() to tell next valid
parameter's position. This fix the issue in case command length in input is not
equal to its name's length such as "help|?", and the case input start with
space such as "  s". 
  7/7: better commit message.

v4:
  Address Eric's comments:
  1/7, 2/7, 4/7: better commit title and message.
  1/7 remove useless (char *) in old code, add space around "for ()" in old code.
  3/7: separate code moving patch before usage.
  4/7: add space around "for ()" in old code, add min(nb_args, MAX_ARGS) in free
to make code stronger.

v5:
  4/7: use "a < b ? a : b" instead of macro min.

v6:
  Address Luiz's comments:
  1/13 ~ 5/13: splitted small patches.
  5/13: added commit message about the correctness of replacing of cur_mon and
test result.
  6/13: better comments in code.
  7/13: added commit message about the reason of code moving.
  8/13: new patch to improve parse_cmdline(), since it is a more generic
function now.
  9/13: reworked the commit message, better commentes in code, use
free_cmdline_args() in clean. It is a bit hard to split this patch into
smaller meaning ful ones, so kepted this patch as a relative larger one,
with better commit message.
  12/13: put case 'S' with case 's' in monitor_find_completion_by_table().
moved this patch ahead of patch 13/13.
  13/13: this patch is moved behind patch 12/13.

  Generic change:
  10/13: splitted patch which moved out the reentrant part into a separate
function, make review easier. This also avoided re-parsing the command line
which does in previous version.
  11/13: splitted patch, which simply remove usage of info_cmds and support
sub command by re-enter the function.

v7:
  Address Luiz's comments:
  5/13: moved the comments why the change is safe, to cover-letter.
  8/13: use assert in free_cmdline_args(), fail when args in input exceed
the limit in parse_cmdline().

v8:
  Address Eric's comments:
  Fix typo in commit messages.

V9:
  General:
  6: new patch, I found that call can be optimized, so move it.
  Address Luiz's comments:
  7: split function to init the monitor.
  8: move the cmd_table init code to parse_cmdlinethe splitted function.
  11: directly return in fail calling parse_cmdline(), in help_cmd_dump().
  Other:
  After discuss with Luiz, it would be good to add test case for qmp fd-add
related stuff. Current test case is 045, which doesn't cover the case
that fd is added by SCM at runtime, and that case is related to monitor.
I have drafted a version, which can pass both on upstream and this series,
but it may require a bit more code, so send it later as another series.

v10:
  11: remove clean label in help_cmd_dump().

Wenchao Xia (15):
  1 monitor: avoid use of global *cur_mon in cmd_completion()
  2 monitor: avoid use of global *cur_mon in file_completion()
  3 monitor: avoid use of global *cur_mon in block_completion_it()
  4 monitor: avoid use of global *cur_mon in monitor_find_completion()
  5 monitor: avoid use of global *cur_mon in readline_completion()
  6 monitor: call sortcmdlist() only one time
  7 monitor: split off monitor_data_init()
  8 monitor: avoid direct use of global variable *mon_cmds
  9 monitor: code move for parse_cmdline()
  10 monitor: refine parse_cmdline()
  11 monitor: support sub command in help
  12 monitor: refine monitor_find_completion()
  13 monitor: support sub command in auto completion
  14 monitor: allow "help" show message for single command in sub group
  15 monitor: improve auto complete of "help" for single command in sub group

 hmp-commands.hx            |    2 +-
 include/monitor/readline.h |    3 +-
 monitor.c                  |  459 ++++++++++++++++++++++++++++----------------
 readline.c                 |    5 +-
 4 files changed, 303 insertions(+), 166 deletions(-)

[Qemu-devel] [PATCH V10 01/15] monitor: avoid use of global *cur_mon in cmd_completion()

[Wenchao Xia]

A new local variable *mon is added in monitor_find_completion()
to make compile pass, which will be removed later in
conversion patch for monitor_find_completion().

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/monitor.c b/monitor.c
index ee9744c..31b527d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4008,7 +4008,7 @@ out:
     QDECREF(qdict);
 }
 
-static void cmd_completion(const char *name, const char *list)
+static void cmd_completion(Monitor *mon, const char *name, const char *list)
 {
     const char *p, *pstart;
     char cmd[128];
@@ -4026,7 +4026,7 @@ static void cmd_completion(const char *name, const char *list)
         memcpy(cmd, pstart, len);
         cmd[len] = '\0';
         if (name[0] == '\0' || !strncmp(name, cmd, strlen(name))) {
-            readline_add_completion(cur_mon->rs, cmd);
+            readline_add_completion(mon->rs, cmd);
         }
         if (*p == '\0')
             break;
@@ -4140,6 +4140,7 @@ static void monitor_find_completion(const char *cmdline)
     int nb_args, i, len;
     const char *ptype, *str;
     const mon_cmd_t *cmd;
+    Monitor *mon = cur_mon;
 
     parse_cmdline(cmdline, &nb_args, args);
 #ifdef DEBUG_COMPLETION
@@ -4165,7 +4166,7 @@ static void monitor_find_completion(const char *cmdline)
             cmdname = args[0];
         readline_set_completion_index(cur_mon->rs, strlen(cmdname));
         for(cmd = mon_cmds; cmd->name != NULL; cmd++) {
-            cmd_completion(cmdname, cmd->name);
+            cmd_completion(mon, cmdname, cmd->name);
         }
     } else {
         /* find the command */
@@ -4206,7 +4207,7 @@ static void monitor_find_completion(const char *cmdline)
             if (!strcmp(cmd->name, "info")) {
                 readline_set_completion_index(cur_mon->rs, strlen(str));
                 for(cmd = info_cmds; cmd->name != NULL; cmd++) {
-                    cmd_completion(str, cmd->name);
+                    cmd_completion(mon, str, cmd->name);
                 }
             } else if (!strcmp(cmd->name, "sendkey")) {
                 char *sep = strrchr(str, '-');
@@ -4214,12 +4215,12 @@ static void monitor_find_completion(const char *cmdline)
                     str = sep + 1;
                 readline_set_completion_index(cur_mon->rs, strlen(str));
                 for (i = 0; i < Q_KEY_CODE_MAX; i++) {
-                    cmd_completion(str, QKeyCode_lookup[i]);
+                    cmd_completion(mon, str, QKeyCode_lookup[i]);
                 }
             } else if (!strcmp(cmd->name, "help|?")) {
                 readline_set_completion_index(cur_mon->rs, strlen(str));
                 for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
-                    cmd_completion(str, cmd->name);
+                    cmd_completion(mon, str, cmd->name);
                 }
             }
             break;
-- 
1.7.1

[Qemu-devel] [PATCH V10 02/15] monitor: avoid use of global *cur_mon in file_completion()

[Wenchao Xia]

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 31b527d..a59a402 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4034,7 +4034,7 @@ static void cmd_completion(Monitor *mon, const char *name, const char *list)
     }
 }
 
-static void file_completion(const char *input)
+static void file_completion(Monitor *mon, const char *input)
 {
     DIR *ffs;
     struct dirent *d;
@@ -4057,7 +4057,7 @@ static void file_completion(const char *input)
         pstrcpy(file_prefix, sizeof(file_prefix), p + 1);
     }
 #ifdef DEBUG_COMPLETION
-    monitor_printf(cur_mon, "input='%s' path='%s' prefix='%s'\n",
+    monitor_printf(mon, "input='%s' path='%s' prefix='%s'\n",
                    input, path, file_prefix);
 #endif
     ffs = opendir(path);
@@ -4084,7 +4084,7 @@ static void file_completion(const char *input)
             if (stat(file, &sb) == 0 && S_ISDIR(sb.st_mode)) {
                 pstrcat(file, sizeof(file), "/");
             }
-            readline_add_completion(cur_mon->rs, file);
+            readline_add_completion(mon->rs, file);
         }
     }
     closedir(ffs);
@@ -4195,7 +4195,7 @@ static void monitor_find_completion(const char *cmdline)
         case 'F':
             /* file completion */
             readline_set_completion_index(cur_mon->rs, strlen(str));
-            file_completion(str);
+            file_completion(mon, str);
             break;
         case 'B':
             /* block device name completion */
-- 
1.7.1

[Qemu-devel] [PATCH V10 03/15] monitor: avoid use of global *cur_mon in block_completion_it()

[Wenchao Xia]

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index a59a402..d1a6bda 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4090,14 +4090,21 @@ static void file_completion(Monitor *mon, const char *input)
     closedir(ffs);
 }
 
+typedef struct MonitorBlockComplete {
+    Monitor *mon;
+    const char *input;
+} MonitorBlockComplete;
+
 static void block_completion_it(void *opaque, BlockDriverState *bs)
 {
     const char *name = bdrv_get_device_name(bs);
-    const char *input = opaque;
+    MonitorBlockComplete *mbc = opaque;
+    Monitor *mon = mbc->mon;
+    const char *input = mbc->input;
 
     if (input[0] == '\0' ||
         !strncmp(name, (char *)input, strlen(input))) {
-        readline_add_completion(cur_mon->rs, name);
+        readline_add_completion(mon->rs, name);
     }
 }
 
@@ -4141,6 +4148,7 @@ static void monitor_find_completion(const char *cmdline)
     const char *ptype, *str;
     const mon_cmd_t *cmd;
     Monitor *mon = cur_mon;
+    MonitorBlockComplete mbs;
 
     parse_cmdline(cmdline, &nb_args, args);
 #ifdef DEBUG_COMPLETION
@@ -4199,8 +4207,10 @@ static void monitor_find_completion(const char *cmdline)
             break;
         case 'B':
             /* block device name completion */
-            readline_set_completion_index(cur_mon->rs, strlen(str));
-            bdrv_iterate(block_completion_it, (void *)str);
+            mbs.mon = mon;
+            mbs.input = str;
+            readline_set_completion_index(mon->rs, strlen(str));
+            bdrv_iterate(block_completion_it, &mbs);
             break;
         case 's':
             /* XXX: more generic ? */
-- 
1.7.1

[Qemu-devel] [PATCH V10 04/15] monitor: avoid use of global *cur_mon in monitor_find_completion()

[Wenchao Xia]

Parameter *mon is added, and local variable *mon added in previous patch
is removed. The caller readline_completion(), pass rs->mon as value, which
should be initialized in readline_init() called by monitor_init().

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 include/monitor/readline.h |    3 ++-
 monitor.c                  |   18 +++++++++---------
 readline.c                 |    2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/include/monitor/readline.h b/include/monitor/readline.h
index fc9806e..0faf6e1 100644
--- a/include/monitor/readline.h
+++ b/include/monitor/readline.h
@@ -8,7 +8,8 @@
 #define READLINE_MAX_COMPLETIONS 256
 
 typedef void ReadLineFunc(Monitor *mon, const char *str, void *opaque);
-typedef void ReadLineCompletionFunc(const char *cmdline);
+typedef void ReadLineCompletionFunc(Monitor *mon,
+                                    const char *cmdline);
 
 typedef struct ReadLineState {
     char cmd_buf[READLINE_CMD_BUF_SIZE + 1];
diff --git a/monitor.c b/monitor.c
index d1a6bda..fffb434 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4140,20 +4140,20 @@ static const char *next_arg_type(const char *typestr)
     return (p != NULL ? ++p : typestr);
 }
 
-static void monitor_find_completion(const char *cmdline)
+static void monitor_find_completion(Monitor *mon,
+                                    const char *cmdline)
 {
     const char *cmdname;
     char *args[MAX_ARGS];
     int nb_args, i, len;
     const char *ptype, *str;
     const mon_cmd_t *cmd;
-    Monitor *mon = cur_mon;
     MonitorBlockComplete mbs;
 
     parse_cmdline(cmdline, &nb_args, args);
 #ifdef DEBUG_COMPLETION
-    for(i = 0; i < nb_args; i++) {
-        monitor_printf(cur_mon, "arg%d = '%s'\n", i, (char *)args[i]);
+    for (i = 0; i < nb_args; i++) {
+        monitor_printf(mon, "arg%d = '%s'\n", i, args[i]);
     }
 #endif
 
@@ -4172,7 +4172,7 @@ static void monitor_find_completion(const char *cmdline)
             cmdname = "";
         else
             cmdname = args[0];
-        readline_set_completion_index(cur_mon->rs, strlen(cmdname));
+        readline_set_completion_index(mon->rs, strlen(cmdname));
         for(cmd = mon_cmds; cmd->name != NULL; cmd++) {
             cmd_completion(mon, cmdname, cmd->name);
         }
@@ -4202,7 +4202,7 @@ static void monitor_find_completion(const char *cmdline)
         switch(*ptype) {
         case 'F':
             /* file completion */
-            readline_set_completion_index(cur_mon->rs, strlen(str));
+            readline_set_completion_index(mon->rs, strlen(str));
             file_completion(mon, str);
             break;
         case 'B':
@@ -4215,7 +4215,7 @@ static void monitor_find_completion(const char *cmdline)
         case 's':
             /* XXX: more generic ? */
             if (!strcmp(cmd->name, "info")) {
-                readline_set_completion_index(cur_mon->rs, strlen(str));
+                readline_set_completion_index(mon->rs, strlen(str));
                 for(cmd = info_cmds; cmd->name != NULL; cmd++) {
                     cmd_completion(mon, str, cmd->name);
                 }
@@ -4223,12 +4223,12 @@ static void monitor_find_completion(const char *cmdline)
                 char *sep = strrchr(str, '-');
                 if (sep)
                     str = sep + 1;
-                readline_set_completion_index(cur_mon->rs, strlen(str));
+                readline_set_completion_index(mon->rs, strlen(str));
                 for (i = 0; i < Q_KEY_CODE_MAX; i++) {
                     cmd_completion(mon, str, QKeyCode_lookup[i]);
                 }
             } else if (!strcmp(cmd->name, "help|?")) {
-                readline_set_completion_index(cur_mon->rs, strlen(str));
+                readline_set_completion_index(mon->rs, strlen(str));
                 for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
                     cmd_completion(mon, str, cmd->name);
                 }
diff --git a/readline.c b/readline.c
index 1c0f7ee..c91b324 100644
--- a/readline.c
+++ b/readline.c
@@ -285,7 +285,7 @@ static void readline_completion(ReadLineState *rs)
     cmdline = g_malloc(rs->cmd_buf_index + 1);
     memcpy(cmdline, rs->cmd_buf, rs->cmd_buf_index);
     cmdline[rs->cmd_buf_index] = '\0';
-    rs->completion_finder(cmdline);
+    rs->completion_finder(rs->mon, cmdline);
     g_free(cmdline);
 
     /* no completion found */
-- 
1.7.1

[Qemu-devel] [PATCH V10 05/15] monitor: avoid use of global *cur_mon in readline_completion()

[Wenchao Xia]

Now all completion functions do not use *cur_mon any more, instead
they use rs->mon. In short, structure ReadLineState decide where
the complete action would be taken now.

Tested with the case that qemu have two telnet monitors, auto
completion function works normal.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 readline.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/readline.c b/readline.c
index c91b324..abf27dd 100644
--- a/readline.c
+++ b/readline.c
@@ -276,7 +276,6 @@ void readline_set_completion_index(ReadLineState *rs, int index)
 
 static void readline_completion(ReadLineState *rs)
 {
-    Monitor *mon = cur_mon;
     int len, i, j, max_width, nb_cols, max_prefix;
     char *cmdline;
 
@@ -300,7 +299,7 @@ static void readline_completion(ReadLineState *rs)
         if (len > 0 && rs->completions[0][len - 1] != '/')
             readline_insert_char(rs, ' ');
     } else {
-        monitor_printf(mon, "\n");
+        monitor_printf(rs->mon, "\n");
         max_width = 0;
         max_prefix = 0;	
         for(i = 0; i < rs->nb_completions; i++) {
-- 
1.7.1

[Qemu-devel] [PATCH V10 06/15] monitor: call sortcmdlist() only one time

[Wenchao Xia]

It doesn't need to be done for every monitor, so change it.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 monitor.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index fffb434..db4d441 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4762,6 +4762,7 @@ void monitor_init(CharDriverState *chr, int flags)
 
     if (is_first_init) {
         monitor_protocol_event_init();
+        sortcmdlist();
         is_first_init = 0;
     }
 
@@ -4791,8 +4792,6 @@ void monitor_init(CharDriverState *chr, int flags)
     QLIST_INSERT_HEAD(&mon_list, mon, entry);
     if (!default_mon || (flags & MONITOR_IS_DEFAULT))
         default_mon = mon;
-
-    sortcmdlist();
 }
 
 static void bdrv_password_cb(Monitor *mon, const char *password, void *opaque)
-- 
1.7.1

[Qemu-devel] [PATCH V10 07/15] monitor: split off monitor_data_init()

[Wenchao Xia]

In qmp_human_monitor_command(), the monitor need to initialized for
basic functionalities, and later more init code will be added, so
split off this function. Note that it is different with QMP mode
monitor which accept json string from monitor's input,
qmp_human_monitor_command() retrieve the human style command from
QMP input, then send the command to a normal mode monitor.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 monitor.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/monitor.c b/monitor.c
index db4d441..bd02112 100644
--- a/monitor.c
+++ b/monitor.c
@@ -683,14 +683,24 @@ static int do_qmp_capabilities(Monitor *mon, const QDict *params,
 
 static void handle_user_command(Monitor *mon, const char *cmdline);
 
+static void monitor_data_init(Monitor *mon)
+{
+    memset(mon, 0, sizeof(Monitor));
+    mon->outbuf = qstring_new();
+}
+
+static void monitor_data_destroy(Monitor *mon)
+{
+    QDECREF(mon->outbuf);
+}
+
 char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
                                 int64_t cpu_index, Error **errp)
 {
     char *output = NULL;
     Monitor *old_mon, hmp;
 
-    memset(&hmp, 0, sizeof(hmp));
-    hmp.outbuf = qstring_new();
+    monitor_data_init(&hmp);
     hmp.skip_flush = true;
 
     old_mon = cur_mon;
@@ -716,7 +726,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
     }
 
 out:
-    QDECREF(hmp.outbuf);
+    monitor_data_destroy(&hmp);
     return output;
 }
 
@@ -4766,8 +4776,8 @@ void monitor_init(CharDriverState *chr, int flags)
         is_first_init = 0;
     }
 
-    mon = g_malloc0(sizeof(*mon));
-    mon->outbuf = qstring_new();
+    mon = g_malloc(sizeof(*mon));
+    monitor_data_init(mon);
 
     mon->chr = chr;
     mon->flags = flags;
-- 
1.7.1

[Qemu-devel] [PATCH V10 08/15] monitor: avoid direct use of global variable *mon_cmds

[Wenchao Xia]

New member *cmd_table is added in structure Monitor to avoid direct usage of
*mon_cmds. Now monitor have an associated command table, when global variable
*info_cmds is also discarded, structure Monitor would gain full control about
how to deal with user input.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 monitor.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/monitor.c b/monitor.c
index bd02112..c5fb6ce 100644
--- a/monitor.c
+++ b/monitor.c
@@ -195,6 +195,7 @@ struct Monitor {
     CPUState *mon_cpu;
     BlockDriverCompletionFunc *password_completion_cb;
     void *password_opaque;
+    mon_cmd_t *cmd_table;
     QError *error;
     QLIST_HEAD(,mon_fd_t) fds;
     QLIST_ENTRY(Monitor) entry;
@@ -687,6 +688,8 @@ static void monitor_data_init(Monitor *mon)
 {
     memset(mon, 0, sizeof(Monitor));
     mon->outbuf = qstring_new();
+    /* Use *mon_cmds by default. */
+    mon->cmd_table = mon_cmds;
 }
 
 static void monitor_data_destroy(Monitor *mon)
@@ -767,7 +770,7 @@ static void help_cmd(Monitor *mon, const char *name)
     if (name && !strcmp(name, "info")) {
         help_cmd_dump(mon, info_cmds, "info ", NULL);
     } else {
-        help_cmd_dump(mon, mon_cmds, "", name);
+        help_cmd_dump(mon, mon->cmd_table, "", name);
         if (name && !strcmp(name, "log")) {
             const QEMULogItem *item;
             monitor_printf(mon, "Log items (comma separated):\n");
@@ -3994,7 +3997,7 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
 
     qdict = qdict_new();
 
-    cmd = monitor_parse_command(mon, cmdline, 0, mon_cmds, qdict);
+    cmd = monitor_parse_command(mon, cmdline, 0, mon->cmd_table, qdict);
     if (!cmd)
         goto out;
 
@@ -4183,12 +4186,12 @@ static void monitor_find_completion(Monitor *mon,
         else
             cmdname = args[0];
         readline_set_completion_index(mon->rs, strlen(cmdname));
-        for(cmd = mon_cmds; cmd->name != NULL; cmd++) {
+        for (cmd = mon->cmd_table; cmd->name != NULL; cmd++) {
             cmd_completion(mon, cmdname, cmd->name);
         }
     } else {
         /* find the command */
-        for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
+        for (cmd = mon->cmd_table; cmd->name != NULL; cmd++) {
             if (compare_cmd(args[0], cmd->name)) {
                 break;
             }
@@ -4239,7 +4242,7 @@ static void monitor_find_completion(Monitor *mon,
                 }
             } else if (!strcmp(cmd->name, "help|?")) {
                 readline_set_completion_index(mon->rs, strlen(str));
-                for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
+                for (cmd = mon->cmd_table; cmd->name != NULL; cmd++) {
                     cmd_completion(mon, str, cmd->name);
                 }
             }
-- 
1.7.1

[Qemu-devel] [PATCH V10 09/15] monitor: code move for parse_cmdline()

[Wenchao Xia]

help_cmd() need this function later, so move it. get_str() is called by
parse_cmdline() so it is moved also. Some code style error reported by
check script, is also fixed.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c |  191 +++++++++++++++++++++++++++++++------------------------------
 1 files changed, 98 insertions(+), 93 deletions(-)

diff --git a/monitor.c b/monitor.c
index c5fb6ce..46191d3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -753,6 +753,104 @@ static int compare_cmd(const char *name, const char *list)
     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:
+                    qemu_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 != '\"') {
+            qemu_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
+
+/* NOTE: this parser is an approximate form of the real command parser */
+static void 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) {
+            break;
+        }
+        ret = get_str(buf, sizeof(buf), &p);
+        args[nb_args] = g_strdup(buf);
+        nb_args++;
+        if (ret < 0) {
+            break;
+        }
+    }
+    *pnb_args = nb_args;
+}
+
 static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds,
                           const char *prefix, const char *name)
 {
@@ -3433,71 +3531,6 @@ static int get_double(Monitor *mon, double *pval, const char **pp)
     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:
-                    qemu_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 != '\"') {
-            qemu_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;
-}
-
 /*
  * Store the command-name in cmdname, and return a pointer to
  * the remaining of the command string.
@@ -3554,8 +3587,6 @@ static char *key_get_info(const char *type, char **key)
 static int default_fmt_format = 'x';
 static int default_fmt_size = 4;
 
-#define MAX_ARGS 16
-
 static int is_valid_option(const char *c, const char *typestr)
 {
     char option[3];
@@ -4121,32 +4152,6 @@ static void block_completion_it(void *opaque, BlockDriverState *bs)
     }
 }
 
-/* NOTE: this parser is an approximate form of the real command parser */
-static void 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)
-            break;
-        ret = get_str(buf, sizeof(buf), &p);
-        args[nb_args] = g_strdup(buf);
-        nb_args++;
-        if (ret < 0)
-            break;
-    }
-    *pnb_args = nb_args;
-}
-
 static const char *next_arg_type(const char *typestr)
 {
     const char *p = strchr(typestr, ':');
-- 
1.7.1

[Qemu-devel] [PATCH V10 10/15] monitor: refine parse_cmdline()

[Wenchao Xia]

Since this function will be used by help_cmd() later, so improve
it to make it more generic and easier to use. free_cmdline_args()
is added too as paired function to free the result.

One change of this function is that, when the valid args in input
exceed the limit of MAX_ARGS, it fails now, instead of return with
MAX_ARGS of parsed args in old code. This should not impact much
since it is rare that user input many args in monitor's "help" and
auto complete scenario.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c |   51 ++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/monitor.c b/monitor.c
index 46191d3..2a9dfb2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -821,9 +821,33 @@ static int get_str(char *buf, int buf_size, const char **pp)
 
 #define MAX_ARGS 16
 
-/* NOTE: this parser is an approximate form of the real command parser */
-static void parse_cmdline(const char *cmdline,
-                          int *pnb_args, char **args)
+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;
@@ -839,16 +863,21 @@ static void parse_cmdline(const char *cmdline,
             break;
         }
         if (nb_args >= MAX_ARGS) {
-            break;
+            goto fail;
         }
         ret = get_str(buf, sizeof(buf), &p);
-        args[nb_args] = g_strdup(buf);
-        nb_args++;
         if (ret < 0) {
-            break;
+            goto fail;
         }
+        args[nb_args] = g_strdup(buf);
+        nb_args++;
     }
     *pnb_args = nb_args;
+    return 0;
+
+ fail:
+    free_cmdline_args(args, nb_args);
+    return -1;
 }
 
 static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds,
@@ -4168,7 +4197,9 @@ static void monitor_find_completion(Monitor *mon,
     const mon_cmd_t *cmd;
     MonitorBlockComplete mbs;
 
-    parse_cmdline(cmdline, &nb_args, args);
+    if (parse_cmdline(cmdline, &nb_args, args) < 0) {
+        return;
+    }
 #ifdef DEBUG_COMPLETION
     for (i = 0; i < nb_args; i++) {
         monitor_printf(mon, "arg%d = '%s'\n", i, args[i]);
@@ -4258,9 +4289,7 @@ static void monitor_find_completion(Monitor *mon,
     }
 
 cleanup:
-    for (i = 0; i < nb_args; i++) {
-        g_free(args[i]);
-    }
+    free_cmdline_args(args, nb_args);
 }
 
 static int monitor_can_read(void *opaque)
-- 
1.7.1

[Qemu-devel] [PATCH V10 11/15] monitor: support sub command in help

[Wenchao Xia]

The old code in help_cmd() uses global 'info_cmds' and treats it as a
special case. Actually 'info_cmds' is a sub command group of 'mon_cmds',
in order to avoid direct use of it, help_cmd() needs to change its work
mechanism to support sub command and not treat it as a special case
any more.

To support sub command, help_cmd() will first parse the input and then call
help_cmd_dump(), which works as a reentrant function. When it meets a sub
command, it simply enters the function again. Since help dumping needs to
know whole input to printf full help message include prefix, for example,
"help info block" need to printf prefix "info", so help_cmd_dump() takes all
args from input and extra parameter arg_index to identify the progress.
Another function help_cmd_dump_one() is introduced to printf the prefix
and command's help message.

Now help supports sub command, so later if another sub command group is
added in any depth, help will automatically work for it. Still "help info
block" will show error since command parser reject additional parameter,
which can be improved later. "log" is still treated as a special case.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 monitor.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/monitor.c b/monitor.c
index 2a9dfb2..c8855d2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -880,33 +880,75 @@ static int parse_cmdline(const char *cmdline,
     return -1;
 }
 
+static void help_cmd_dump_one(Monitor *mon,
+                              const mon_cmd_t *cmd,
+                              char **prefix_args,
+                              int prefix_args_nb)
+{
+    int i;
+
+    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 mon_cmd_t *cmds,
-                          const char *prefix, const char *name)
+                          char **args, int nb_args, int arg_index)
 {
     const mon_cmd_t *cmd;
 
-    for(cmd = cmds; cmd->name != NULL; cmd++) {
-        if (!name || !strcmp(name, cmd->name))
-            monitor_printf(mon, "%s%s %s -- %s\n", prefix, cmd->name,
-                           cmd->params, cmd->help);
+    /* 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)) {
+            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);
+            }
+            break;
+        }
     }
 }
 
 static void help_cmd(Monitor *mon, const char *name)
 {
-    if (name && !strcmp(name, "info")) {
-        help_cmd_dump(mon, info_cmds, "info ", NULL);
-    } else {
-        help_cmd_dump(mon, mon->cmd_table, "", name);
-        if (name && !strcmp(name, "log")) {
+    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, mon->cmd_table, args, nb_args, 0);
+
+    free_cmdline_args(args, nb_args);
 }
 
 static void do_help_cmd(Monitor *mon, const QDict *qdict)
-- 
1.7.1

[Qemu-devel] [PATCH V10 12/15] monitor: refine monitor_find_completion()

[Wenchao Xia]

In order to support sub command in auto completion, a reentrant function
is needed, so monitor_find_completion() is split into two parts. The
first part does parsing of user input which need to be done only once,
the second part does the auto completion job according to the parsing
result, which contains the necessary code to support sub command and
works as the reentrant function. The global "info_cmds" is still used
in second part, which will be replaced by sub command code later.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c |   65 ++++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/monitor.c b/monitor.c
index c8855d2..9d1b0cb 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4229,34 +4229,17 @@ static const char *next_arg_type(const char *typestr)
     return (p != NULL ? ++p : typestr);
 }
 
-static void monitor_find_completion(Monitor *mon,
-                                    const char *cmdline)
+static void monitor_find_completion_by_table(Monitor *mon,
+                                             const mon_cmd_t *cmd_table,
+                                             char **args,
+                                             int nb_args)
 {
     const char *cmdname;
-    char *args[MAX_ARGS];
-    int nb_args, i, len;
+    int i;
     const char *ptype, *str;
     const mon_cmd_t *cmd;
     MonitorBlockComplete mbs;
 
-    if (parse_cmdline(cmdline, &nb_args, args) < 0) {
-        return;
-    }
-#ifdef DEBUG_COMPLETION
-    for (i = 0; i < nb_args; i++) {
-        monitor_printf(mon, "arg%d = '%s'\n", i, args[i]);
-    }
-#endif
-
-    /* if the line ends with a space, it means we want to complete the
-       next arg */
-    len = strlen(cmdline);
-    if (len > 0 && qemu_isspace(cmdline[len - 1])) {
-        if (nb_args >= MAX_ARGS) {
-            goto cleanup;
-        }
-        args[nb_args++] = g_strdup("");
-    }
     if (nb_args <= 1) {
         /* command completion */
         if (nb_args == 0)
@@ -4264,18 +4247,18 @@ static void monitor_find_completion(Monitor *mon,
         else
             cmdname = args[0];
         readline_set_completion_index(mon->rs, strlen(cmdname));
-        for (cmd = mon->cmd_table; cmd->name != NULL; cmd++) {
+        for (cmd = cmd_table; cmd->name != NULL; cmd++) {
             cmd_completion(mon, cmdname, cmd->name);
         }
     } else {
         /* find the command */
-        for (cmd = mon->cmd_table; cmd->name != NULL; cmd++) {
+        for (cmd = cmd_table; cmd->name != NULL; cmd++) {
             if (compare_cmd(args[0], cmd->name)) {
                 break;
             }
         }
         if (!cmd->name) {
-            goto cleanup;
+            return;
         }
 
         ptype = next_arg_type(cmd->args_type);
@@ -4320,7 +4303,7 @@ static void monitor_find_completion(Monitor *mon,
                 }
             } else if (!strcmp(cmd->name, "help|?")) {
                 readline_set_completion_index(mon->rs, strlen(str));
-                for (cmd = mon->cmd_table; cmd->name != NULL; cmd++) {
+                for (cmd = cmd_table; cmd->name != NULL; cmd++) {
                     cmd_completion(mon, str, cmd->name);
                 }
             }
@@ -4329,6 +4312,36 @@ static void monitor_find_completion(Monitor *mon,
             break;
         }
     }
+}
+
+static void monitor_find_completion(Monitor *mon,
+                                    const char *cmdline)
+{
+    char *args[MAX_ARGS];
+    int nb_args, len;
+
+    /* 1. parse the cmdline */
+    if (parse_cmdline(cmdline, &nb_args, args) < 0) {
+        return;
+    }
+#ifdef DEBUG_COMPLETION
+    for (i = 0; i < nb_args; i++) {
+        monitor_printf(mon, "arg%d = '%s'\n", i, args[i]);
+    }
+#endif
+
+    /* if the line ends with a space, it means we want to complete the
+       next arg */
+    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, mon->cmd_table, args, nb_args);
 
 cleanup:
     free_cmdline_args(args, nb_args);
-- 
1.7.1

[Qemu-devel] [PATCH V10 13/15] monitor: support sub command in auto completion

[Wenchao Xia]

This patch allows auto completion work normal for sub command case,
"info block [DEVICE]" can auto complete now, by re-enter the completion
function. In original code "info" is treated as a special case, now it
is treated as a sub command group, global variable info_cmds is not used
any more.

"help" command is still treated as a special case, since it is not a sub
command group but want to auto complete command in root command table.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/monitor.c b/monitor.c
index 9d1b0cb..d3f357c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4261,6 +4261,12 @@ static void monitor_find_completion_by_table(Monitor *mon,
             return;
         }
 
+        if (cmd->sub_table) {
+            /* do the job again */
+            return monitor_find_completion_by_table(mon, cmd->sub_table,
+                                                    &args[1], nb_args - 1);
+        }
+
         ptype = next_arg_type(cmd->args_type);
         for(i = 0; i < nb_args - 2; i++) {
             if (*ptype != '\0') {
@@ -4287,13 +4293,7 @@ static void monitor_find_completion_by_table(Monitor *mon,
             bdrv_iterate(block_completion_it, &mbs);
             break;
         case 's':
-            /* XXX: more generic ? */
-            if (!strcmp(cmd->name, "info")) {
-                readline_set_completion_index(mon->rs, strlen(str));
-                for(cmd = info_cmds; cmd->name != NULL; cmd++) {
-                    cmd_completion(mon, str, cmd->name);
-                }
-            } else if (!strcmp(cmd->name, "sendkey")) {
+            if (!strcmp(cmd->name, "sendkey")) {
                 char *sep = strrchr(str, '-');
                 if (sep)
                     str = sep + 1;
-- 
1.7.1

[Qemu-devel] [PATCH V10 14/15] monitor: allow "help" show message for single command in sub group

[Wenchao Xia]

A new parameter type 'S' is introduced to allow user input any string.
"help info block" works normal now.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 hmp-commands.hx |    2 +-
 monitor.c       |   27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8c6b91a..c161fe9 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 d3f357c..81f41e6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -83,6 +83,7 @@
  * 'F'          filename
  * 'B'          block device name
  * 's'          string (accept optional quote)
+ * 'S'          it just appends the rest of the 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.
@@ -4046,6 +4047,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);
@@ -4293,6 +4319,7 @@ static void monitor_find_completion_by_table(Monitor *mon,
             bdrv_iterate(block_completion_it, &mbs);
             break;
         case 's':
+        case 'S':
             if (!strcmp(cmd->name, "sendkey")) {
                 char *sep = strrchr(str, '-');
                 if (sep)
-- 
1.7.1

[Qemu-devel] [PATCH V10 15/15] monitor: improve auto complete of "help" for single command in sub group

[Wenchao Xia]

Now special case "help *" in auto completion can work with sub commands,
such as "help info u*".

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 81f41e6..8d07f9c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4329,10 +4329,8 @@ static void monitor_find_completion_by_table(Monitor *mon,
                     cmd_completion(mon, str, QKeyCode_lookup[i]);
                 }
             } else if (!strcmp(cmd->name, "help|?")) {
-                readline_set_completion_index(mon->rs, strlen(str));
-                for (cmd = cmd_table; cmd->name != NULL; cmd++) {
-                    cmd_completion(mon, str, cmd->name);
-                }
+                monitor_find_completion_by_table(mon, cmd_table,
+                                                 &args[1], nb_args - 1);
             }
             break;
         default:
-- 
1.7.1

Re: [Qemu-devel] [PATCH V10 00/15] monitor: support sub command group in auto completion and help

[Luiz Capitulino]

On Tue, 27 Aug 2013 20:38:12 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> This series make auto completion and help functions works normal for sub
> command, by using reentrant functions. In order to do that, global variables
> are not directly used in those functions any more. With this series, cmd_table
> is a member of structure Monitor so it is possible to create a monitor with
> different command table now, auto completion will work in that monitor. In
> short, "info" is not treated as a special case now, this series ensure help
> and auto complete function works normal for any sub command added in the future.
> 
> Patch 5 replaced cur_mon with rs->mon, it is safe because:
> monitor_init() calls readline_init() which initialize mon->rs, result is
> mon->rs->mon == mon. Then qemu_chr_add_handlers() is called, which make
> monitor_read() function take *mon as its opaque. Later, when user input,
> monitor_read() is called, where cur_mon is set to *mon by "cur_mon = opaque".
> If qemu's monitors run in one thread, then later in readline_handle_byte()
> and readline_comletion(), cur_mon is actually equal to rs->mon, in another
> word, it points to the monitor instance, so it is safe to replace *cur_mon
> in those functions.
> 
> Thanks for Luiz and Eric for reviewing.

Applied to the qmp branch, thanks.

Btw, I'm assuming that the patches that contain Eric's reviewed-by didn't
change since he reviewed them. Personally, I don't think it's right to carry
reviewed-bys when the patch changes, even if it's something small or
obvious.

Re: [Qemu-devel] [PATCH V10 00/15] monitor: support sub command group in auto completion and help

[Wenchao Xia]

于 2013-8-29 22:16, Luiz Capitulino 写道:
> On Tue, 27 Aug 2013 20:38:12 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> This series make auto completion and help functions works normal for sub
>> command, by using reentrant functions. In order to do that, global variables
>> are not directly used in those functions any more. With this series, cmd_table
>> is a member of structure Monitor so it is possible to create a monitor with
>> different command table now, auto completion will work in that monitor. In
>> short, "info" is not treated as a special case now, this series ensure help
>> and auto complete function works normal for any sub command added in the future.
>>
>> Patch 5 replaced cur_mon with rs->mon, it is safe because:
>> monitor_init() calls readline_init() which initialize mon->rs, result is
>> mon->rs->mon == mon. Then qemu_chr_add_handlers() is called, which make
>> monitor_read() function take *mon as its opaque. Later, when user input,
>> monitor_read() is called, where cur_mon is set to *mon by "cur_mon = opaque".
>> If qemu's monitors run in one thread, then later in readline_handle_byte()
>> and readline_comletion(), cur_mon is actually equal to rs->mon, in another
>> word, it points to the monitor instance, so it is safe to replace *cur_mon
>> in those functions.
>>
>> Thanks for Luiz and Eric for reviewing.
>
> Applied to the qmp branch, thanks.
>
> Btw, I'm assuming that the patches that contain Eric's reviewed-by didn't
> change since he reviewed them. Personally, I don't think it's right to carry
> reviewed-bys when the patch changes, even if it's something small or
> obvious.
>
   I remember that the "Reviewed-by" were removed on all changed
patches, except:
   Patches of V7 have typo issues which were pointed
out by Eric with "Reviewed-by", for those I fixed the typo and kept the
"Reviewed by".
   Some patches were not modified but the line number may change,
since previous patch may have inserted new lines. For those I didn't
remove "Reviewed-by".
   If the above is not appropriate, I'll avoid it.

-- 
Best Regards

Wenchao Xia

Re: [Qemu-devel] [PATCH V10 00/15] monitor: support sub command group in auto completion and help

[Luiz Capitulino]

On Fri, 30 Aug 2013 10:22:43 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> 于 2013-8-29 22:16, Luiz Capitulino 写道:
> > On Tue, 27 Aug 2013 20:38:12 +0800
> > Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> >
> >> This series make auto completion and help functions works normal for sub
> >> command, by using reentrant functions. In order to do that, global variables
> >> are not directly used in those functions any more. With this series, cmd_table
> >> is a member of structure Monitor so it is possible to create a monitor with
> >> different command table now, auto completion will work in that monitor. In
> >> short, "info" is not treated as a special case now, this series ensure help
> >> and auto complete function works normal for any sub command added in the future.
> >>
> >> Patch 5 replaced cur_mon with rs->mon, it is safe because:
> >> monitor_init() calls readline_init() which initialize mon->rs, result is
> >> mon->rs->mon == mon. Then qemu_chr_add_handlers() is called, which make
> >> monitor_read() function take *mon as its opaque. Later, when user input,
> >> monitor_read() is called, where cur_mon is set to *mon by "cur_mon = opaque".
> >> If qemu's monitors run in one thread, then later in readline_handle_byte()
> >> and readline_comletion(), cur_mon is actually equal to rs->mon, in another
> >> word, it points to the monitor instance, so it is safe to replace *cur_mon
> >> in those functions.
> >>
> >> Thanks for Luiz and Eric for reviewing.
> >
> > Applied to the qmp branch, thanks.
> >
> > Btw, I'm assuming that the patches that contain Eric's reviewed-by didn't
> > change since he reviewed them. Personally, I don't think it's right to carry
> > reviewed-bys when the patch changes, even if it's something small or
> > obvious.
> >
>    I remember that the "Reviewed-by" were removed on all changed
> patches, except:
>    Patches of V7 have typo issues which were pointed
> out by Eric with "Reviewed-by", for those I fixed the typo and kept the
> "Reviewed by".
>    Some patches were not modified but the line number may change,
> since previous patch may have inserted new lines. For those I didn't
> remove "Reviewed-by".
>    If the above is not appropriate, I'll avoid it.

Looks fine to me.