All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help
@ 2013-08-23  8:17 Wenchao Xia
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 01/15] monitor: avoid use of global *cur_mon in cmd_completion() Wenchao Xia
                   ` (15 more replies)
  0 siblings, 16 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:17 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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.

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                  |  460 ++++++++++++++++++++++++++++---------------
 readline.c                 |    5 +-
 4 files changed, 304 insertions(+), 166 deletions(-)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 01/15] monitor: avoid use of global *cur_mon in cmd_completion()
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
@ 2013-08-23  8:17 ` Wenchao Xia
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 02/15] monitor: avoid use of global *cur_mon in file_completion() Wenchao Xia
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:17 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 da9c9a2..e0154a8 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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 02/15] monitor: avoid use of global *cur_mon in file_completion()
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 01/15] monitor: avoid use of global *cur_mon in cmd_completion() Wenchao Xia
@ 2013-08-23  8:17 ` Wenchao Xia
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 03/15] monitor: avoid use of global *cur_mon in block_completion_it() Wenchao Xia
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:17 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 e0154a8..30819fa 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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 03/15] monitor: avoid use of global *cur_mon in block_completion_it()
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 01/15] monitor: avoid use of global *cur_mon in cmd_completion() Wenchao Xia
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 02/15] monitor: avoid use of global *cur_mon in file_completion() Wenchao Xia
@ 2013-08-23  8:17 ` Wenchao Xia
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 04/15] monitor: avoid use of global *cur_mon in monitor_find_completion() Wenchao Xia
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:17 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 30819fa..6171c75 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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 04/15] monitor: avoid use of global *cur_mon in monitor_find_completion()
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (2 preceding siblings ...)
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 03/15] monitor: avoid use of global *cur_mon in block_completion_it() Wenchao Xia
@ 2013-08-23  8:17 ` Wenchao Xia
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 05/15] monitor: avoid use of global *cur_mon in readline_completion() Wenchao Xia
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:17 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 6171c75..63a779f 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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 05/15] monitor: avoid use of global *cur_mon in readline_completion()
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (3 preceding siblings ...)
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 04/15] monitor: avoid use of global *cur_mon in monitor_find_completion() Wenchao Xia
@ 2013-08-23  8:17 ` Wenchao Xia
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 06/15] monitor: call sortcmdlist() only one time Wenchao Xia
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:17 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 06/15] monitor: call sortcmdlist() only one time
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (4 preceding siblings ...)
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 05/15] monitor: avoid use of global *cur_mon in readline_completion() Wenchao Xia
@ 2013-08-23  8:17 ` Wenchao Xia
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 07/15] monitor: split off monitor_data_init() Wenchao Xia
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:17 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 63a779f..bf019e2 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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 07/15] monitor: split off monitor_data_init()
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (5 preceding siblings ...)
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 06/15] monitor: call sortcmdlist() only one time Wenchao Xia
@ 2013-08-23  8:17 ` Wenchao Xia
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 08/15] monitor: avoid direct use of global variable *mon_cmds Wenchao Xia
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:17 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 bf019e2..3ac98ef 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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 08/15] monitor: avoid direct use of global variable *mon_cmds
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (6 preceding siblings ...)
  2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 07/15] monitor: split off monitor_data_init() Wenchao Xia
@ 2013-08-23  8:18 ` Wenchao Xia
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 09/15] monitor: code move for parse_cmdline() Wenchao Xia
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:18 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 3ac98ef..8152607 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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 09/15] monitor: code move for parse_cmdline()
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (7 preceding siblings ...)
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 08/15] monitor: avoid direct use of global variable *mon_cmds Wenchao Xia
@ 2013-08-23  8:18 ` Wenchao Xia
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 10/15] monitor: refine parse_cmdline() Wenchao Xia
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:18 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 8152607..1db0117 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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 10/15] monitor: refine parse_cmdline()
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (8 preceding siblings ...)
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 09/15] monitor: code move for parse_cmdline() Wenchao Xia
@ 2013-08-23  8:18 ` Wenchao Xia
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 11/15] monitor: support sub command in help Wenchao Xia
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:18 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 1db0117..8897291 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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 11/15] monitor: support sub command in help
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (9 preceding siblings ...)
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 10/15] monitor: refine parse_cmdline() Wenchao Xia
@ 2013-08-23  8:18 ` Wenchao Xia
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 12/15] monitor: refine monitor_find_completion() Wenchao Xia
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:18 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/monitor.c b/monitor.c
index 8897291..1ace29e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -880,33 +880,76 @@ 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);
+
+cleanup:
+    free_cmdline_args(args, nb_args);
 }
 
 static void do_help_cmd(Monitor *mon, const QDict *qdict)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 12/15] monitor: refine monitor_find_completion()
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (10 preceding siblings ...)
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 11/15] monitor: support sub command in help Wenchao Xia
@ 2013-08-23  8:18 ` Wenchao Xia
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 13/15] monitor: support sub command in auto completion Wenchao Xia
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:18 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 1ace29e..49a5a88 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4230,34 +4230,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)
@@ -4265,18 +4248,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);
@@ -4321,7 +4304,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);
                 }
             }
@@ -4330,6 +4313,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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 13/15] monitor: support sub command in auto completion
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (11 preceding siblings ...)
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 12/15] monitor: refine monitor_find_completion() Wenchao Xia
@ 2013-08-23  8:18 ` Wenchao Xia
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 14/15] monitor: allow "help" show message for single command in sub group Wenchao Xia
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:18 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 49a5a88..47791b4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4262,6 +4262,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') {
@@ -4288,13 +4294,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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 14/15] monitor: allow "help" show message for single command in sub group
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (12 preceding siblings ...)
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 13/15] monitor: support sub command in auto completion Wenchao Xia
@ 2013-08-23  8:18 ` Wenchao Xia
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 15/15] monitor: improve auto complete of "help" " Wenchao Xia
  2013-08-26 15:55 ` [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Luiz Capitulino
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:18 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 47791b4..b657c85 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.
@@ -4047,6 +4048,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);
@@ -4294,6 +4320,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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH V9 15/15] monitor: improve auto complete of "help" for single command in sub group
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (13 preceding siblings ...)
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 14/15] monitor: allow "help" show message for single command in sub group Wenchao Xia
@ 2013-08-23  8:18 ` Wenchao Xia
  2013-08-26 15:55 ` [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Luiz Capitulino
  15 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-23  8:18 UTC (permalink / raw
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

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 b657c85..62fc433 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4330,10 +4330,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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help
  2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
                   ` (14 preceding siblings ...)
  2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 15/15] monitor: improve auto complete of "help" " Wenchao Xia
@ 2013-08-26 15:55 ` Luiz Capitulino
  2013-08-27  1:43   ` Wenchao Xia
  15 siblings, 1 reply; 21+ messages in thread
From: Luiz Capitulino @ 2013-08-26 15:55 UTC (permalink / raw
  To: Wenchao Xia; +Cc: pbonzini, qemu-devel, armbru

On Fri, 23 Aug 2013 16:17:52 +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.

This one doesn't even build :(

/home/lcapitulino/work/src/upstream/qmp-unstable/monitor.c: In function ‘help_cmd’:
/home/lcapitulino/work/src/upstream/qmp-unstable/monitor.c:952:1: error: label ‘cleanup’ defined but not used [-Werror=unused-label]
cc1: all warnings being treated as errors
make[1]: *** [monitor.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [subdir-x86_64-softmmu] Error 2

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help
  2013-08-26 15:55 ` [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Luiz Capitulino
@ 2013-08-27  1:43   ` Wenchao Xia
  2013-08-27 12:39     ` Wenchao Xia
  0 siblings, 1 reply; 21+ messages in thread
From: Wenchao Xia @ 2013-08-27  1:43 UTC (permalink / raw
  To: Luiz Capitulino; +Cc: pbonzini, qemu-devel, armbru

于 2013-8-26 23:55, Luiz Capitulino 写道:
> On Fri, 23 Aug 2013 16:17:52 +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.
>
> This one doesn't even build :(
>
> /home/lcapitulino/work/src/upstream/qmp-unstable/monitor.c: In function ‘help_cmd’:
> /home/lcapitulino/work/src/upstream/qmp-unstable/monitor.c:952:1: error: label ‘cleanup’ defined but not used [-Werror=unused-label]
> cc1: all warnings being treated as errors
> make[1]: *** [monitor.o] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [subdir-x86_64-softmmu] Error 2
>
   Sorry for it, I missed that CC flag in my configure, will fix.
It would be great if you can share your configure settings.

-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help
  2013-08-27  1:43   ` Wenchao Xia
@ 2013-08-27 12:39     ` Wenchao Xia
  2013-08-27 13:19       ` Luiz Capitulino
  0 siblings, 1 reply; 21+ messages in thread
From: Wenchao Xia @ 2013-08-27 12:39 UTC (permalink / raw
  To: Luiz Capitulino; +Cc: pbonzini, qemu-devel, armbru

于 2013-8-27 9:43, Wenchao Xia 写道:
> 于 2013-8-26 23:55, Luiz Capitulino 写道:
>> On Fri, 23 Aug 2013 16:17:52 +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.
>>
>> This one doesn't even build :(
>>
>> /home/lcapitulino/work/src/upstream/qmp-unstable/monitor.c: In
>> function ‘help_cmd’:
>> /home/lcapitulino/work/src/upstream/qmp-unstable/monitor.c:952:1:
>> error: label ‘cleanup’ defined but not used [-Werror=unused-label]
>> cc1: all warnings being treated as errors
>> make[1]: *** [monitor.o] Error 1
>> make[1]: *** Waiting for unfinished jobs....
>> make: *** [subdir-x86_64-softmmu] Error 2
>>
>    Sorry for it, I missed that CC flag in my configure, will fix.
> It would be great if you can share your configure settings.
>
   Respined v10 and tested with -Werror=unused-label, hope no other
issue is missed in my test.

-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help
  2013-08-27 12:39     ` Wenchao Xia
@ 2013-08-27 13:19       ` Luiz Capitulino
  2013-08-28  2:16         ` Wenchao Xia
  0 siblings, 1 reply; 21+ messages in thread
From: Luiz Capitulino @ 2013-08-27 13:19 UTC (permalink / raw
  To: Wenchao Xia; +Cc: pbonzini, qemu-devel, armbru

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

> 于 2013-8-27 9:43, Wenchao Xia 写道:
> > 于 2013-8-26 23:55, Luiz Capitulino 写道:
> >> On Fri, 23 Aug 2013 16:17:52 +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.
> >>
> >> This one doesn't even build :(
> >>
> >> /home/lcapitulino/work/src/upstream/qmp-unstable/monitor.c: In
> >> function ‘help_cmd’:
> >> /home/lcapitulino/work/src/upstream/qmp-unstable/monitor.c:952:1:
> >> error: label ‘cleanup’ defined but not used [-Werror=unused-label]
> >> cc1: all warnings being treated as errors
> >> make[1]: *** [monitor.o] Error 1
> >> make[1]: *** Waiting for unfinished jobs....
> >> make: *** [subdir-x86_64-softmmu] Error 2
> >>
> >    Sorry for it, I missed that CC flag in my configure, will fix.
> > It would be great if you can share your configure settings.
> >
>    Respined v10 and tested with -Werror=unused-label, hope no other
> issue is missed in my test.

My configure line doesn't have anything special:

 ../configure --target-list=x86_64-softmmu

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help
  2013-08-27 13:19       ` Luiz Capitulino
@ 2013-08-28  2:16         ` Wenchao Xia
  0 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2013-08-28  2:16 UTC (permalink / raw
  To: Luiz Capitulino; +Cc: pbonzini, qemu-devel, armbru

于 2013-8-27 21:19, Luiz Capitulino 写道:
> On Tue, 27 Aug 2013 20:39:37 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> 于 2013-8-27 9:43, Wenchao Xia 写道:
>>> 于 2013-8-26 23:55, Luiz Capitulino 写道:
>>>> On Fri, 23 Aug 2013 16:17:52 +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.
>>>>
>>>> This one doesn't even build :(
>>>>
>>>> /home/lcapitulino/work/src/upstream/qmp-unstable/monitor.c: In
>>>> function ‘help_cmd’:
>>>> /home/lcapitulino/work/src/upstream/qmp-unstable/monitor.c:952:1:
>>>> error: label ‘cleanup’ defined but not used [-Werror=unused-label]
>>>> cc1: all warnings being treated as errors
>>>> make[1]: *** [monitor.o] Error 1
>>>> make[1]: *** Waiting for unfinished jobs....
>>>> make: *** [subdir-x86_64-softmmu] Error 2
>>>>
>>>     Sorry for it, I missed that CC flag in my configure, will fix.
>>> It would be great if you can share your configure settings.
>>>
>>     Respined v10 and tested with -Werror=unused-label, hope no other
>> issue is missed in my test.
>
> My configure line doesn't have anything special:
>
>   ../configure --target-list=x86_64-softmmu
>
   I used special cc warn flag before, it seems that by default more
flags would be set, will double check with default configure.

-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2013-08-28  2:16 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-23  8:17 [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Wenchao Xia
2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 01/15] monitor: avoid use of global *cur_mon in cmd_completion() Wenchao Xia
2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 02/15] monitor: avoid use of global *cur_mon in file_completion() Wenchao Xia
2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 03/15] monitor: avoid use of global *cur_mon in block_completion_it() Wenchao Xia
2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 04/15] monitor: avoid use of global *cur_mon in monitor_find_completion() Wenchao Xia
2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 05/15] monitor: avoid use of global *cur_mon in readline_completion() Wenchao Xia
2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 06/15] monitor: call sortcmdlist() only one time Wenchao Xia
2013-08-23  8:17 ` [Qemu-devel] [PATCH V9 07/15] monitor: split off monitor_data_init() Wenchao Xia
2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 08/15] monitor: avoid direct use of global variable *mon_cmds Wenchao Xia
2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 09/15] monitor: code move for parse_cmdline() Wenchao Xia
2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 10/15] monitor: refine parse_cmdline() Wenchao Xia
2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 11/15] monitor: support sub command in help Wenchao Xia
2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 12/15] monitor: refine monitor_find_completion() Wenchao Xia
2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 13/15] monitor: support sub command in auto completion Wenchao Xia
2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 14/15] monitor: allow "help" show message for single command in sub group Wenchao Xia
2013-08-23  8:18 ` [Qemu-devel] [PATCH V9 15/15] monitor: improve auto complete of "help" " Wenchao Xia
2013-08-26 15:55 ` [Qemu-devel] [PATCH V9 00/15] monitor: support sub command group in auto completion and help Luiz Capitulino
2013-08-27  1:43   ` Wenchao Xia
2013-08-27 12:39     ` Wenchao Xia
2013-08-27 13:19       ` Luiz Capitulino
2013-08-28  2:16         ` Wenchao Xia

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.