qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] support sub command group for auto completion in monitor
@ 2013-06-21  6:37 Wenchao Xia
  2013-06-21  6:37 ` [Qemu-devel] [PATCH 1/2] monitor: discard global variable in auto completion functions Wenchao Xia
  2013-06-21  6:37 ` [Qemu-devel] [PATCH 2/2] monitor: support sub commands in auto completion Wenchao Xia
  0 siblings, 2 replies; 6+ messages in thread
From: Wenchao Xia @ 2013-06-21  6:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

This patch modified auto completion a bit, to make it work when there is a
folded sub command group, for example, "info" is a sub command group of root
command group.

Note that at patch 1, the parameter *mon and *cmd_table is brought until
monitor_init() level. If *cmd_table is added also in it, a monitor can
be created with different command tables, but that requirement do not exist
yet, so not changed it to save trouble. 

Wenchao Xia (2):
  1) monitor: discard global variable in auto completion functions
  2) monitor: support sub commands in auto completion

 include/monitor/readline.h |    8 ++++-
 monitor.c                  |   78 ++++++++++++++++++++++++++------------------
 readline.c                 |    4 ++-
 3 files changed, 56 insertions(+), 34 deletions(-)

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

* [Qemu-devel] [PATCH 1/2] monitor: discard global variable in auto completion functions
  2013-06-21  6:37 [Qemu-devel] [PATCH 0/2] support sub command group for auto completion in monitor Wenchao Xia
@ 2013-06-21  6:37 ` Wenchao Xia
  2013-06-21 15:24   ` Luiz Capitulino
  2013-06-21  6:37 ` [Qemu-devel] [PATCH 2/2] monitor: support sub commands in auto completion Wenchao Xia
  1 sibling, 1 reply; 6+ messages in thread
From: Wenchao Xia @ 2013-06-21  6:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

In monitor_find_completion() and related functions, Global variable
*mon_cmds is not used any more, make them reenterable safely.
*cur_mon is also not used now. *info_cmds is still there, but soon
will be removed by a new way of sub command completion.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 include/monitor/readline.h |    8 ++++-
 monitor.c                  |   70 ++++++++++++++++++++++++++-----------------
 readline.c                 |    4 ++-
 3 files changed, 52 insertions(+), 30 deletions(-)

diff --git a/include/monitor/readline.h b/include/monitor/readline.h
index fc9806e..a4c7bce 100644
--- a/include/monitor/readline.h
+++ b/include/monitor/readline.h
@@ -7,8 +7,12 @@
 #define READLINE_MAX_CMDS 64
 #define READLINE_MAX_COMPLETIONS 256
 
+typedef struct mon_cmd_t mon_cmd_t;
+
 typedef void ReadLineFunc(Monitor *mon, const char *str, void *opaque);
-typedef void ReadLineCompletionFunc(const char *cmdline);
+typedef void ReadLineCompletionFunc(Monitor *mon,
+                                    mon_cmd_t *cmd_table,
+                                    const char *cmdline);
 
 typedef struct ReadLineState {
     char cmd_buf[READLINE_CMD_BUF_SIZE + 1];
@@ -35,6 +39,7 @@ typedef struct ReadLineState {
     int read_password;
     char prompt[256];
     Monitor *mon;
+    mon_cmd_t *cmd_table;
 } ReadLineState;
 
 void readline_add_completion(ReadLineState *rs, const char *str);
@@ -50,6 +55,7 @@ void readline_restart(ReadLineState *rs);
 void readline_show_prompt(ReadLineState *rs);
 
 ReadLineState *readline_init(Monitor *mon,
+                             mon_cmd_t *cmd_table,
                              ReadLineCompletionFunc *completion_finder);
 
 #endif /* !READLINE_H */
diff --git a/monitor.c b/monitor.c
index 70ae8f5..bc60171 100644
--- a/monitor.c
+++ b/monitor.c
@@ -116,7 +116,7 @@ struct MonitorCompletionData {
     void (*user_print)(Monitor *mon, const QObject *data);
 };
 
-typedef struct mon_cmd_t {
+struct mon_cmd_t {
     const char *name;
     const char *args_type;
     const char *params;
@@ -134,7 +134,7 @@ typedef struct mon_cmd_t {
      * used, and mhandler of 1st level plays the role of help function.
      */
     struct mon_cmd_t *sub_table;
-} mon_cmd_t;
+};
 
 /* file descriptors passed via SCM_RIGHTS */
 typedef struct mon_fd_t mon_fd_t;
@@ -3999,7 +3999,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];
@@ -4017,7 +4017,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;
@@ -4025,7 +4025,7 @@ static void cmd_completion(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;
@@ -4048,7 +4048,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);
@@ -4075,20 +4075,28 @@ 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);
 }
 
-static void block_completion_it(void *opaque, BlockDriverState *bs)
+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);
     }
 }
 
@@ -4124,18 +4132,21 @@ 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,
+                                    mon_cmd_t *cmd_table,
+                                    const char *cmdline)
 {
     const char *cmdname;
     char *args[MAX_ARGS];
     int nb_args, i, len;
     const char *ptype, *str;
     const mon_cmd_t *cmd;
+    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]);
+        monitor_printf(mon, "arg%d = '%s'\n", i, (char *)args[i]);
     }
 #endif
 
@@ -4154,13 +4165,13 @@ static void monitor_find_completion(const char *cmdline)
             cmdname = "";
         else
             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);
+        readline_set_completion_index(mon->rs, strlen(cmdname));
+        for (cmd = 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 = cmd_table; cmd->name != NULL; cmd++) {
             if (compare_cmd(args[0], cmd->name)) {
                 break;
             }
@@ -4184,33 +4195,35 @@ static void monitor_find_completion(const char *cmdline)
         switch(*ptype) {
         case 'F':
             /* file completion */
-            readline_set_completion_index(cur_mon->rs, strlen(str));
-            file_completion(str);
+            readline_set_completion_index(mon->rs, strlen(str));
+            file_completion(mon, str);
             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 ? */
             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(str, cmd->name);
+                    cmd_completion(mon, str, cmd->name);
                 }
             } else if (!strcmp(cmd->name, "sendkey")) {
                 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(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);
+                readline_set_completion_index(mon->rs, strlen(str));
+                for (cmd = cmd_table; cmd->name != NULL; cmd++) {
+                    cmd_completion(mon, str, cmd->name);
                 }
             }
             break;
@@ -4751,7 +4764,8 @@ void monitor_init(CharDriverState *chr, int flags)
     mon->chr = chr;
     mon->flags = flags;
     if (flags & MONITOR_USE_READLINE) {
-        mon->rs = readline_init(mon, monitor_find_completion);
+        /* Always use *mon_cmds as entry now. */
+        mon->rs = readline_init(mon, mon_cmds, monitor_find_completion);
         monitor_read_command(mon, 0);
     }
 
diff --git a/readline.c b/readline.c
index 1c0f7ee..d97e062 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, rs->cmd_table, cmdline);
     g_free(cmdline);
 
     /* no completion found */
@@ -482,12 +482,14 @@ const char *readline_get_history(ReadLineState *rs, unsigned int index)
 }
 
 ReadLineState *readline_init(Monitor *mon,
+                             mon_cmd_t *cmd_table,
                              ReadLineCompletionFunc *completion_finder)
 {
     ReadLineState *rs = g_malloc0(sizeof(*rs));
 
     rs->hist_entry = -1;
     rs->mon = mon;
+    rs->cmd_table = cmd_table;
     rs->completion_finder = completion_finder;
 
     return rs;
-- 
1.7.1

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

* [Qemu-devel] [PATCH 2/2] monitor: support sub commands in auto completion
  2013-06-21  6:37 [Qemu-devel] [PATCH 0/2] support sub command group for auto completion in monitor Wenchao Xia
  2013-06-21  6:37 ` [Qemu-devel] [PATCH 1/2] monitor: discard global variable in auto completion functions Wenchao Xia
@ 2013-06-21  6:37 ` Wenchao Xia
  2013-06-21 15:24   ` Luiz Capitulino
  1 sibling, 1 reply; 6+ messages in thread
From: Wenchao Xia @ 2013-06-21  6:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Wenchao Xia, armbru, lcapitulino

This patch allow auot completion work normal in sub command case,
"info block [DEVICE]" can auto complete now, by re-enter the completion
function. Also, original "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.

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

diff --git a/monitor.c b/monitor.c
index bc60171..c706644 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4180,6 +4180,11 @@ static void monitor_find_completion(Monitor *mon,
             goto cleanup;
         }
 
+        if (cmd->sub_table) {
+            return monitor_find_completion(mon, cmd->sub_table,
+                                           cmdline + strlen(cmd->name));
+        }
+
         ptype = next_arg_type(cmd->args_type);
         for(i = 0; i < nb_args - 2; i++) {
             if (*ptype != '\0') {
@@ -4207,12 +4212,7 @@ static void monitor_find_completion(Monitor *mon,
             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] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] monitor: discard global variable in auto completion functions
  2013-06-21  6:37 ` [Qemu-devel] [PATCH 1/2] monitor: discard global variable in auto completion functions Wenchao Xia
@ 2013-06-21 15:24   ` Luiz Capitulino
  2013-06-24  5:36     ` Wenchao Xia
  0 siblings, 1 reply; 6+ messages in thread
From: Luiz Capitulino @ 2013-06-21 15:24 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: pbonzini, qemu-devel, armbru

On Fri, 21 Jun 2013 14:37:37 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> In monitor_find_completion() and related functions, Global variable
> *mon_cmds is not used any more, make them reenterable safely.
> *cur_mon is also not used now. *info_cmds is still there, but soon
> will be removed by a new way of sub command completion.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  include/monitor/readline.h |    8 ++++-
>  monitor.c                  |   70 ++++++++++++++++++++++++++-----------------
>  readline.c                 |    4 ++-
>  3 files changed, 52 insertions(+), 30 deletions(-)
> 
> diff --git a/include/monitor/readline.h b/include/monitor/readline.h
> index fc9806e..a4c7bce 100644
> --- a/include/monitor/readline.h
> +++ b/include/monitor/readline.h
> @@ -7,8 +7,12 @@
>  #define READLINE_MAX_CMDS 64
>  #define READLINE_MAX_COMPLETIONS 256
>  
> +typedef struct mon_cmd_t mon_cmd_t;

This should be in monitor.h. Maybe you could move struct mon_cmd_t there
too.

Also, can you split this series is smaller patches? It looks good, but
it isn't easy to review because it changes too much code at once.

Otherwise this is a nice feature!

> +
>  typedef void ReadLineFunc(Monitor *mon, const char *str, void *opaque);
> -typedef void ReadLineCompletionFunc(const char *cmdline);
> +typedef void ReadLineCompletionFunc(Monitor *mon,
> +                                    mon_cmd_t *cmd_table,
> +                                    const char *cmdline);
>  
>  typedef struct ReadLineState {
>      char cmd_buf[READLINE_CMD_BUF_SIZE + 1];
> @@ -35,6 +39,7 @@ typedef struct ReadLineState {
>      int read_password;
>      char prompt[256];
>      Monitor *mon;
> +    mon_cmd_t *cmd_table;
>  } ReadLineState;
>  
>  void readline_add_completion(ReadLineState *rs, const char *str);
> @@ -50,6 +55,7 @@ void readline_restart(ReadLineState *rs);
>  void readline_show_prompt(ReadLineState *rs);
>  
>  ReadLineState *readline_init(Monitor *mon,
> +                             mon_cmd_t *cmd_table,
>                               ReadLineCompletionFunc *completion_finder);
>  
>  #endif /* !READLINE_H */
> diff --git a/monitor.c b/monitor.c
> index 70ae8f5..bc60171 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -116,7 +116,7 @@ struct MonitorCompletionData {
>      void (*user_print)(Monitor *mon, const QObject *data);
>  };
>  
> -typedef struct mon_cmd_t {
> +struct mon_cmd_t {
>      const char *name;
>      const char *args_type;
>      const char *params;
> @@ -134,7 +134,7 @@ typedef struct mon_cmd_t {
>       * used, and mhandler of 1st level plays the role of help function.
>       */
>      struct mon_cmd_t *sub_table;
> -} mon_cmd_t;
> +};
>  
>  /* file descriptors passed via SCM_RIGHTS */
>  typedef struct mon_fd_t mon_fd_t;
> @@ -3999,7 +3999,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];
> @@ -4017,7 +4017,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;
> @@ -4025,7 +4025,7 @@ static void cmd_completion(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;
> @@ -4048,7 +4048,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);
> @@ -4075,20 +4075,28 @@ 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);
>  }
>  
> -static void block_completion_it(void *opaque, BlockDriverState *bs)
> +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);
>      }
>  }
>  
> @@ -4124,18 +4132,21 @@ 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,
> +                                    mon_cmd_t *cmd_table,
> +                                    const char *cmdline)
>  {
>      const char *cmdname;
>      char *args[MAX_ARGS];
>      int nb_args, i, len;
>      const char *ptype, *str;
>      const mon_cmd_t *cmd;
> +    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]);
> +        monitor_printf(mon, "arg%d = '%s'\n", i, (char *)args[i]);
>      }
>  #endif
>  
> @@ -4154,13 +4165,13 @@ static void monitor_find_completion(const char *cmdline)
>              cmdname = "";
>          else
>              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);
> +        readline_set_completion_index(mon->rs, strlen(cmdname));
> +        for (cmd = 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 = cmd_table; cmd->name != NULL; cmd++) {
>              if (compare_cmd(args[0], cmd->name)) {
>                  break;
>              }
> @@ -4184,33 +4195,35 @@ static void monitor_find_completion(const char *cmdline)
>          switch(*ptype) {
>          case 'F':
>              /* file completion */
> -            readline_set_completion_index(cur_mon->rs, strlen(str));
> -            file_completion(str);
> +            readline_set_completion_index(mon->rs, strlen(str));
> +            file_completion(mon, str);
>              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 ? */
>              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(str, cmd->name);
> +                    cmd_completion(mon, str, cmd->name);
>                  }
>              } else if (!strcmp(cmd->name, "sendkey")) {
>                  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(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);
> +                readline_set_completion_index(mon->rs, strlen(str));
> +                for (cmd = cmd_table; cmd->name != NULL; cmd++) {
> +                    cmd_completion(mon, str, cmd->name);
>                  }
>              }
>              break;
> @@ -4751,7 +4764,8 @@ void monitor_init(CharDriverState *chr, int flags)
>      mon->chr = chr;
>      mon->flags = flags;
>      if (flags & MONITOR_USE_READLINE) {
> -        mon->rs = readline_init(mon, monitor_find_completion);
> +        /* Always use *mon_cmds as entry now. */
> +        mon->rs = readline_init(mon, mon_cmds, monitor_find_completion);
>          monitor_read_command(mon, 0);
>      }
>  
> diff --git a/readline.c b/readline.c
> index 1c0f7ee..d97e062 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, rs->cmd_table, cmdline);
>      g_free(cmdline);
>  
>      /* no completion found */
> @@ -482,12 +482,14 @@ const char *readline_get_history(ReadLineState *rs, unsigned int index)
>  }
>  
>  ReadLineState *readline_init(Monitor *mon,
> +                             mon_cmd_t *cmd_table,
>                               ReadLineCompletionFunc *completion_finder)
>  {
>      ReadLineState *rs = g_malloc0(sizeof(*rs));
>  
>      rs->hist_entry = -1;
>      rs->mon = mon;
> +    rs->cmd_table = cmd_table;
>      rs->completion_finder = completion_finder;
>  
>      return rs;

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

* Re: [Qemu-devel] [PATCH 2/2] monitor: support sub commands in auto completion
  2013-06-21  6:37 ` [Qemu-devel] [PATCH 2/2] monitor: support sub commands in auto completion Wenchao Xia
@ 2013-06-21 15:24   ` Luiz Capitulino
  0 siblings, 0 replies; 6+ messages in thread
From: Luiz Capitulino @ 2013-06-21 15:24 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: pbonzini, qemu-devel, armbru

On Fri, 21 Jun 2013 14:37:38 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> This patch allow auot completion work normal in sub command case,
> "info block [DEVICE]" can auto complete now, by re-enter the completion
> function. Also, original "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.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  monitor.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index bc60171..c706644 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4180,6 +4180,11 @@ static void monitor_find_completion(Monitor *mon,
>              goto cleanup;
>          }
>  
> +        if (cmd->sub_table) {
> +            return monitor_find_completion(mon, cmd->sub_table,
> +                                           cmdline + strlen(cmd->name));
> +        }
> +
>          ptype = next_arg_type(cmd->args_type);
>          for(i = 0; i < nb_args - 2; i++) {
>              if (*ptype != '\0') {
> @@ -4207,12 +4212,7 @@ static void monitor_find_completion(Monitor *mon,
>              break;
>          case 's':
>              /* XXX: more generic ? */

I think you can drop this comment too.

> -            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;

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

* Re: [Qemu-devel] [PATCH 1/2] monitor: discard global variable in auto completion functions
  2013-06-21 15:24   ` Luiz Capitulino
@ 2013-06-24  5:36     ` Wenchao Xia
  0 siblings, 0 replies; 6+ messages in thread
From: Wenchao Xia @ 2013-06-24  5:36 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: pbonzini, qemu-devel, armbru

于 2013-6-21 23:24, Luiz Capitulino 写道:
> On Fri, 21 Jun 2013 14:37:37 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> In monitor_find_completion() and related functions, Global variable
>> *mon_cmds is not used any more, make them reenterable safely.
>> *cur_mon is also not used now. *info_cmds is still there, but soon
>> will be removed by a new way of sub command completion.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   include/monitor/readline.h |    8 ++++-
>>   monitor.c                  |   70 ++++++++++++++++++++++++++-----------------
>>   readline.c                 |    4 ++-
>>   3 files changed, 52 insertions(+), 30 deletions(-)
>>
>> diff --git a/include/monitor/readline.h b/include/monitor/readline.h
>> index fc9806e..a4c7bce 100644
>> --- a/include/monitor/readline.h
>> +++ b/include/monitor/readline.h
>> @@ -7,8 +7,12 @@
>>   #define READLINE_MAX_CMDS 64
>>   #define READLINE_MAX_COMPLETIONS 256
>>
>> +typedef struct mon_cmd_t mon_cmd_t;
>
> This should be in monitor.h. Maybe you could move struct mon_cmd_t there
> too.
>
 >
   Originally monitor.h include readline.h, readline.h need this def in
this patch, so putting it in monitor.h require readline.h include
monitor.h, break build. Do you have a better way to solve it? introduce
monitor-def.h?

   about struct mon_cmd_t, I think it is good to hide it in monitor.c.

> Also, can you split this series is smaller patches? It looks good, but
> it isn't easy to review because it changes too much code at once.
>
   Sure, let me try.

> Otherwise this is a nice feature!
>
>> +
>>   typedef void ReadLineFunc(Monitor *mon, const char *str, void *opaque);
>> -typedef void ReadLineCompletionFunc(const char *cmdline);
>> +typedef void ReadLineCompletionFunc(Monitor *mon,
>> +                                    mon_cmd_t *cmd_table,
>> +                                    const char *cmdline);
>>
>>   typedef struct ReadLineState {
>>       char cmd_buf[READLINE_CMD_BUF_SIZE + 1];
>> @@ -35,6 +39,7 @@ typedef struct ReadLineState {
>>       int read_password;
>>       char prompt[256];
>>       Monitor *mon;
>> +    mon_cmd_t *cmd_table;
>>   } ReadLineState;
>>
>>   void readline_add_completion(ReadLineState *rs, const char *str);
>> @@ -50,6 +55,7 @@ void readline_restart(ReadLineState *rs);
>>   void readline_show_prompt(ReadLineState *rs);
>>
>>   ReadLineState *readline_init(Monitor *mon,
>> +                             mon_cmd_t *cmd_table,
>>                                ReadLineCompletionFunc *completion_finder);
>>
>>   #endif /* !READLINE_H */
>> diff --git a/monitor.c b/monitor.c
>> index 70ae8f5..bc60171 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -116,7 +116,7 @@ struct MonitorCompletionData {
>>       void (*user_print)(Monitor *mon, const QObject *data);
>>   };
>>
>> -typedef struct mon_cmd_t {
>> +struct mon_cmd_t {
>>       const char *name;
>>       const char *args_type;
>>       const char *params;
>> @@ -134,7 +134,7 @@ typedef struct mon_cmd_t {
>>        * used, and mhandler of 1st level plays the role of help function.
>>        */
>>       struct mon_cmd_t *sub_table;
>> -} mon_cmd_t;
>> +};
>>
>>   /* file descriptors passed via SCM_RIGHTS */
>>   typedef struct mon_fd_t mon_fd_t;
>> @@ -3999,7 +3999,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];
>> @@ -4017,7 +4017,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;
>> @@ -4025,7 +4025,7 @@ static void cmd_completion(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;
>> @@ -4048,7 +4048,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);
>> @@ -4075,20 +4075,28 @@ 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);
>>   }
>>
>> -static void block_completion_it(void *opaque, BlockDriverState *bs)
>> +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);
>>       }
>>   }
>>
>> @@ -4124,18 +4132,21 @@ 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,
>> +                                    mon_cmd_t *cmd_table,
>> +                                    const char *cmdline)
>>   {
>>       const char *cmdname;
>>       char *args[MAX_ARGS];
>>       int nb_args, i, len;
>>       const char *ptype, *str;
>>       const mon_cmd_t *cmd;
>> +    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]);
>> +        monitor_printf(mon, "arg%d = '%s'\n", i, (char *)args[i]);
>>       }
>>   #endif
>>
>> @@ -4154,13 +4165,13 @@ static void monitor_find_completion(const char *cmdline)
>>               cmdname = "";
>>           else
>>               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);
>> +        readline_set_completion_index(mon->rs, strlen(cmdname));
>> +        for (cmd = 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 = cmd_table; cmd->name != NULL; cmd++) {
>>               if (compare_cmd(args[0], cmd->name)) {
>>                   break;
>>               }
>> @@ -4184,33 +4195,35 @@ static void monitor_find_completion(const char *cmdline)
>>           switch(*ptype) {
>>           case 'F':
>>               /* file completion */
>> -            readline_set_completion_index(cur_mon->rs, strlen(str));
>> -            file_completion(str);
>> +            readline_set_completion_index(mon->rs, strlen(str));
>> +            file_completion(mon, str);
>>               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 ? */
>>               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(str, cmd->name);
>> +                    cmd_completion(mon, str, cmd->name);
>>                   }
>>               } else if (!strcmp(cmd->name, "sendkey")) {
>>                   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(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);
>> +                readline_set_completion_index(mon->rs, strlen(str));
>> +                for (cmd = cmd_table; cmd->name != NULL; cmd++) {
>> +                    cmd_completion(mon, str, cmd->name);
>>                   }
>>               }
>>               break;
>> @@ -4751,7 +4764,8 @@ void monitor_init(CharDriverState *chr, int flags)
>>       mon->chr = chr;
>>       mon->flags = flags;
>>       if (flags & MONITOR_USE_READLINE) {
>> -        mon->rs = readline_init(mon, monitor_find_completion);
>> +        /* Always use *mon_cmds as entry now. */
>> +        mon->rs = readline_init(mon, mon_cmds, monitor_find_completion);
>>           monitor_read_command(mon, 0);
>>       }
>>
>> diff --git a/readline.c b/readline.c
>> index 1c0f7ee..d97e062 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, rs->cmd_table, cmdline);
>>       g_free(cmdline);
>>
>>       /* no completion found */
>> @@ -482,12 +482,14 @@ const char *readline_get_history(ReadLineState *rs, unsigned int index)
>>   }
>>
>>   ReadLineState *readline_init(Monitor *mon,
>> +                             mon_cmd_t *cmd_table,
>>                                ReadLineCompletionFunc *completion_finder)
>>   {
>>       ReadLineState *rs = g_malloc0(sizeof(*rs));
>>
>>       rs->hist_entry = -1;
>>       rs->mon = mon;
>> +    rs->cmd_table = cmd_table;
>>       rs->completion_finder = completion_finder;
>>
>>       return rs;
>
>


-- 
Best Regards

Wenchao Xia

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

end of thread, other threads:[~2013-06-24  5:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-21  6:37 [Qemu-devel] [PATCH 0/2] support sub command group for auto completion in monitor Wenchao Xia
2013-06-21  6:37 ` [Qemu-devel] [PATCH 1/2] monitor: discard global variable in auto completion functions Wenchao Xia
2013-06-21 15:24   ` Luiz Capitulino
2013-06-24  5:36     ` Wenchao Xia
2013-06-21  6:37 ` [Qemu-devel] [PATCH 2/2] monitor: support sub commands in auto completion Wenchao Xia
2013-06-21 15:24   ` Luiz Capitulino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).