[PATCH 1/3] add static to mi_cmds

[PATCH 1/3] add static to mi_cmds

[Yao Qi]

Hi,
'mi_cmds' is not used out of file mi-cmds.c, so 'static' is added to
it.  It is obvious, and I'll check it in.

gdb:

2012-08-27  Yao Qi  <yao@codesourcery.com>

	* mi/mi-cmds.c (mi_cmds): Add 'static'.
---
 gdb/mi/mi-cmds.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 79fbba1..aebc2b8 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -30,7 +30,7 @@ struct mi_cmd;
 static struct mi_cmd **lookup_table (const char *command);
 static void build_table (struct mi_cmd *commands);
 
-struct mi_cmd mi_cmds[] =
+static struct mi_cmd mi_cmds[] =
 {
   { "ada-task-info", { NULL, 0 }, mi_cmd_ada_task_info },
   { "add-inferior", { NULL, 0 }, mi_cmd_add_inferior },
-- 
1.7.7.6

[PATCH 0/3] Factor code on suppress MI notification

[Yao Qi]

Hi,
When writing new MI notifications, I have to add 'else if (strncmp())'
stuff to suppress each new MI notification, and I don't like this way.
This patch set is to factor the code on notification suppressing.
Patch 3/3 is the major part of this set, which adds a new field
'called' in 'struct mi_cmd', and associate this field of each mi_cmd
to the flag of suppress notification we already have.  It is simple
that when we want to add a new flag to suppress some notifications.

Patch 1/3 is obvious, and I'll apply it.  Patch 2/3 is to define two
new macros to simply the struct initialization in array 'mi_cmds'.

Regression tested on x86_64-linux with native and gdbserver.  OK to
apply?

[PATCH 2/3] new macro DEF_MI_CMD_CLI and DEF_MI_CMD_MI.

[Yao Qi]

Hi,
This patch is to refactor array mi_cmds to use macros to initialize
each struct, and it will be easier to modify 'struct mi_cmd' in my
next patch.

gdb:

2012-08-27  Yao Qi  <yao@codesourcery.com>

	* mi/mi-cmds.c (mi_cmds): New macros DEF_MI_CMD_CLI and
	DEF_MI_CMD_MI.  Adjust it.
---
 gdb/mi/mi-cmds.c |  217 ++++++++++++++++++++++++++++--------------------------
 1 files changed, 112 insertions(+), 105 deletions(-)

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index aebc2b8..0f0d74c 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -32,111 +32,118 @@ static void build_table (struct mi_cmd *commands);
 
 static struct mi_cmd mi_cmds[] =
 {
-  { "ada-task-info", { NULL, 0 }, mi_cmd_ada_task_info },
-  { "add-inferior", { NULL, 0 }, mi_cmd_add_inferior },
-  { "break-after", { "ignore", 1 }, NULL },
-  { "break-condition", { "cond", 1 }, NULL },
-  { "break-commands", { NULL, 0 }, mi_cmd_break_commands },
-  { "break-delete", { "delete breakpoint", 1 }, NULL },
-  { "break-disable", { "disable breakpoint", 1 }, NULL },
-  { "break-enable", { "enable breakpoint", 1 }, NULL },
-  { "break-info", { "info break", 1 }, NULL },
-  { "break-insert", { NULL, 0 }, mi_cmd_break_insert},
-  { "break-list", { "info break", }, NULL },
-  { "break-passcount", { NULL, 0 }, mi_cmd_break_passcount},
-  { "break-watch", { NULL, 0 }, mi_cmd_break_watch},
-  { "data-disassemble", { NULL, 0 }, mi_cmd_disassemble},
-  { "data-evaluate-expression", { NULL, 0 }, mi_cmd_data_evaluate_expression},
-  { "data-list-changed-registers", { NULL, 0 },
-    mi_cmd_data_list_changed_registers},
-  { "data-list-register-names", { NULL, 0 }, mi_cmd_data_list_register_names},
-  { "data-list-register-values", { NULL, 0 },
-    mi_cmd_data_list_register_values},
-  { "data-read-memory", { NULL, 0 }, mi_cmd_data_read_memory},
-  { "data-read-memory-bytes", { NULL, 0 }, mi_cmd_data_read_memory_bytes},
-  { "data-write-memory", { NULL, 0 }, mi_cmd_data_write_memory},
-  { "data-write-memory-bytes", {NULL, 0}, mi_cmd_data_write_memory_bytes},
-  { "data-write-register-values", { NULL, 0 },
-    mi_cmd_data_write_register_values},
-  { "enable-timings", { NULL, 0 }, mi_cmd_enable_timings},
-  { "enable-pretty-printing", { NULL, 0 }, mi_cmd_enable_pretty_printing},
-  { "environment-cd", { NULL, 0 }, mi_cmd_env_cd},
-  { "environment-directory", { NULL, 0 }, mi_cmd_env_dir},
-  { "environment-path", { NULL, 0 }, mi_cmd_env_path},
-  { "environment-pwd", { NULL, 0 }, mi_cmd_env_pwd},
-  { "exec-arguments", { "set args", 1 }, NULL },
-  { "exec-continue", { NULL, 0 }, mi_cmd_exec_continue},
-  { "exec-finish", { NULL, 0 }, mi_cmd_exec_finish},
-  { "exec-jump", { NULL, 0 }, mi_cmd_exec_jump},
-  { "exec-interrupt", { NULL, 0 }, mi_cmd_exec_interrupt},
-  { "exec-next", { NULL, 0 }, mi_cmd_exec_next},
-  { "exec-next-instruction", { NULL, 0 }, mi_cmd_exec_next_instruction},
-  { "exec-return", { NULL, 0 }, mi_cmd_exec_return},
-  { "exec-run", { NULL, 0}, mi_cmd_exec_run},
-  { "exec-step", { NULL, 0 }, mi_cmd_exec_step},
-  { "exec-step-instruction", { NULL, 0 }, mi_cmd_exec_step_instruction},
-  { "exec-until", { "until", 1 }, NULL},
-  { "file-exec-and-symbols", { "file", 1 }, NULL },
-  { "file-exec-file", { "exec-file", 1 }, NULL },
-  { "file-list-exec-source-file", { NULL, 0 },
-    mi_cmd_file_list_exec_source_file},
-  { "file-list-exec-source-files", { NULL, 0 },
-    mi_cmd_file_list_exec_source_files },
-  { "file-symbol-file", { "symbol-file", 1 }, NULL },
-  { "gdb-exit", { NULL, 0 }, mi_cmd_gdb_exit},
-  { "gdb-set", { "set", 1 }, NULL },
-  { "gdb-show", { "show", 1 }, NULL },
-  { "gdb-version", { "show version", 0 }, 0 },
-  { "inferior-tty-set", { NULL, 0 }, mi_cmd_inferior_tty_set},
-  { "inferior-tty-show", { NULL, 0 }, mi_cmd_inferior_tty_show},
-  { "info-os", { NULL, 0 }, mi_cmd_info_os},
-  { "interpreter-exec", { NULL, 0 }, mi_cmd_interpreter_exec},
-  { "list-features", { NULL, 0 }, mi_cmd_list_features},
-  { "list-target-features", { NULL, 0 }, mi_cmd_list_target_features},
-  { "list-thread-groups", { NULL, 0 }, mi_cmd_list_thread_groups },  
-  { "remove-inferior", { NULL, 0 }, mi_cmd_remove_inferior },
-  { "stack-info-depth", { NULL, 0 }, mi_cmd_stack_info_depth},
-  { "stack-info-frame", { NULL, 0 }, mi_cmd_stack_info_frame},
-  { "stack-list-arguments", { NULL, 0 }, mi_cmd_stack_list_args},
-  { "stack-list-frames", { NULL, 0 }, mi_cmd_stack_list_frames},
-  { "stack-list-locals", { NULL, 0 }, mi_cmd_stack_list_locals},
-  { "stack-list-variables", { NULL, 0 }, mi_cmd_stack_list_variables},
-  { "stack-select-frame", { NULL, 0 }, mi_cmd_stack_select_frame},
-  { "symbol-list-lines", { NULL, 0 }, mi_cmd_symbol_list_lines},
-  { "target-attach", { "attach", 1 }, NULL },
-  { "target-detach", { NULL, 0 }, mi_cmd_target_detach },
-  { "target-disconnect", { "disconnect", 0 }, 0 },
-  { "target-download", { "load", 1 }, NULL},
-  { "target-file-delete", { NULL, 0 }, mi_cmd_target_file_delete },
-  { "target-file-get", { NULL, 0 }, mi_cmd_target_file_get },
-  { "target-file-put", { NULL, 0 }, mi_cmd_target_file_put },
-  { "target-select", { "target", 1 }, NULL},
-  { "thread-info", { NULL, 0 }, mi_cmd_thread_info },
-  { "thread-list-ids", { NULL, 0 }, mi_cmd_thread_list_ids},
-  { "thread-select", { NULL, 0 }, mi_cmd_thread_select},
-  { "trace-define-variable", { NULL, 0 }, mi_cmd_trace_define_variable },
-  { "trace-find", { NULL, 0 }, mi_cmd_trace_find },
-  { "trace-list-variables", { NULL, 0 }, mi_cmd_trace_list_variables },
-  { "trace-save", { NULL, 0 }, mi_cmd_trace_save },
-  { "trace-start", { NULL, 0 }, mi_cmd_trace_start },
-  { "trace-status", { NULL, 0 }, mi_cmd_trace_status },
-  { "trace-stop", { NULL, 0 }, mi_cmd_trace_stop },
-  { "var-assign", { NULL, 0 }, mi_cmd_var_assign},
-  { "var-create", { NULL, 0 }, mi_cmd_var_create},
-  { "var-delete", { NULL, 0 }, mi_cmd_var_delete},
-  { "var-evaluate-expression", { NULL, 0 },  mi_cmd_var_evaluate_expression},
-  { "var-info-path-expression", { NULL, 0 }, mi_cmd_var_info_path_expression},
-  { "var-info-expression", { NULL, 0 }, mi_cmd_var_info_expression},
-  { "var-info-num-children", { NULL, 0 }, mi_cmd_var_info_num_children},
-  { "var-info-type", { NULL, 0 }, mi_cmd_var_info_type},
-  { "var-list-children", { NULL, 0 }, mi_cmd_var_list_children},
-  { "var-set-format", { NULL, 0 }, mi_cmd_var_set_format},
-  { "var-set-frozen", { NULL, 0 }, mi_cmd_var_set_frozen},
-  { "var-set-update-range", { NULL, 0 }, mi_cmd_var_set_update_range },
-  { "var-set-visualizer", { NULL, 0 }, mi_cmd_var_set_visualizer},
-  { "var-show-attributes", { NULL, 0 }, mi_cmd_var_show_attributes},
-  { "var-show-format", { NULL, 0 }, mi_cmd_var_show_format},
-  { "var-update", { NULL, 0 }, mi_cmd_var_update},
+/* Define a MI command of NAME, and its corresponding CLI command is
+   CLI_NAME.  */
+#define DEF_MI_CMD_CLI(NAME, CLI_NAME, ARGS_P) \
+  { NAME, { CLI_NAME, ARGS_P}, NULL}
+
+/* Define a MI command of NAME, and implemented by function MI_FUNC.  */
+#define DEF_MI_CMD_MI(NAME, MI_FUNC) { NAME, {NULL, 0}, MI_FUNC }
+
+  DEF_MI_CMD_MI ("ada-task-info", mi_cmd_ada_task_info),
+  DEF_MI_CMD_MI ("add-inferior", mi_cmd_add_inferior),
+  DEF_MI_CMD_CLI ("break-after", "ignore", 1),
+  DEF_MI_CMD_CLI ("break-condition","cond", 1),
+  DEF_MI_CMD_MI ("break-commands", mi_cmd_break_commands),
+  DEF_MI_CMD_CLI ("break-delete", "delete breakpoint", 1),
+  DEF_MI_CMD_CLI ("break-disable", "disable breakpoint", 1),
+  DEF_MI_CMD_CLI ("break-enable", "enable breakpoint", 1),
+  DEF_MI_CMD_CLI ("break-info", "info break", 1),
+  DEF_MI_CMD_MI ("break-insert", mi_cmd_break_insert),
+  DEF_MI_CMD_CLI ("break-list", "info break", 0),
+  DEF_MI_CMD_MI ("break-passcount", mi_cmd_break_passcount),
+  DEF_MI_CMD_MI ("break-watch", mi_cmd_break_watch),
+  DEF_MI_CMD_MI ("data-disassemble", mi_cmd_disassemble),
+  DEF_MI_CMD_MI ("data-evaluate-expression", mi_cmd_data_evaluate_expression),
+  DEF_MI_CMD_MI ("data-list-changed-registers",
+		 mi_cmd_data_list_changed_registers),
+  DEF_MI_CMD_MI ("data-list-register-names", mi_cmd_data_list_register_names),
+  DEF_MI_CMD_MI ("data-list-register-values", mi_cmd_data_list_register_values),
+  DEF_MI_CMD_MI ("data-read-memory", mi_cmd_data_read_memory),
+  DEF_MI_CMD_MI ("data-read-memory-bytes", mi_cmd_data_read_memory_bytes),
+  DEF_MI_CMD_MI ("data-write-memory", mi_cmd_data_write_memory),
+  DEF_MI_CMD_MI ("data-write-memory-bytes", mi_cmd_data_write_memory_bytes),
+  DEF_MI_CMD_MI ("data-write-register-values",
+		 mi_cmd_data_write_register_values),
+  DEF_MI_CMD_MI ("enable-timings", mi_cmd_enable_timings),
+  DEF_MI_CMD_MI ("enable-pretty-printing", mi_cmd_enable_pretty_printing),
+  DEF_MI_CMD_MI ("environment-cd", mi_cmd_env_cd),
+  DEF_MI_CMD_MI ("environment-directory", mi_cmd_env_dir),
+  DEF_MI_CMD_MI ("environment-path", mi_cmd_env_path),
+  DEF_MI_CMD_MI ("environment-pwd", mi_cmd_env_pwd),
+  DEF_MI_CMD_CLI ("exec-arguments", "set args", 1),
+  DEF_MI_CMD_MI ("exec-continue", mi_cmd_exec_continue),
+  DEF_MI_CMD_MI ("exec-finish", mi_cmd_exec_finish),
+  DEF_MI_CMD_MI ("exec-jump", mi_cmd_exec_jump),
+  DEF_MI_CMD_MI ("exec-interrupt", mi_cmd_exec_interrupt),
+  DEF_MI_CMD_MI ("exec-next", mi_cmd_exec_next),
+  DEF_MI_CMD_MI ("exec-next-instruction", mi_cmd_exec_next_instruction),
+  DEF_MI_CMD_MI ("exec-return", mi_cmd_exec_return),
+  DEF_MI_CMD_MI ("exec-run", mi_cmd_exec_run),
+  DEF_MI_CMD_MI ("exec-step", mi_cmd_exec_step),
+  DEF_MI_CMD_MI ("exec-step-instruction", mi_cmd_exec_step_instruction),
+  DEF_MI_CMD_CLI ("exec-until", "until", 1),
+  DEF_MI_CMD_CLI ("file-exec-and-symbols", "file", 1),
+  DEF_MI_CMD_CLI ("file-exec-file", "exec-file", 1),
+  DEF_MI_CMD_MI ("file-list-exec-source-file",
+		 mi_cmd_file_list_exec_source_file),
+  DEF_MI_CMD_MI ("file-list-exec-source-files",
+		 mi_cmd_file_list_exec_source_files),
+  DEF_MI_CMD_CLI ("file-symbol-file", "symbol-file", 1),
+  DEF_MI_CMD_MI ("gdb-exit", mi_cmd_gdb_exit),
+  DEF_MI_CMD_CLI ("gdb-set", "set", 1),
+  DEF_MI_CMD_CLI ("gdb-show", "show", 1),
+  DEF_MI_CMD_CLI ("gdb-version", "show version", 0),
+  DEF_MI_CMD_MI ("inferior-tty-set", mi_cmd_inferior_tty_set),
+  DEF_MI_CMD_MI ("inferior-tty-show", mi_cmd_inferior_tty_show),
+  DEF_MI_CMD_MI ("info-os", mi_cmd_info_os),
+  DEF_MI_CMD_MI ("interpreter-exec", mi_cmd_interpreter_exec),
+  DEF_MI_CMD_MI ("list-features", mi_cmd_list_features),
+  DEF_MI_CMD_MI ("list-target-features", mi_cmd_list_target_features),
+  DEF_MI_CMD_MI ("list-thread-groups", mi_cmd_list_thread_groups),
+  DEF_MI_CMD_MI ("remove-inferior", mi_cmd_remove_inferior),
+  DEF_MI_CMD_MI ("stack-info-depth", mi_cmd_stack_info_depth),
+  DEF_MI_CMD_MI ("stack-info-frame", mi_cmd_stack_info_frame),
+  DEF_MI_CMD_MI ("stack-list-arguments", mi_cmd_stack_list_args),
+  DEF_MI_CMD_MI ("stack-list-frames", mi_cmd_stack_list_frames),
+  DEF_MI_CMD_MI ("stack-list-locals", mi_cmd_stack_list_locals),
+  DEF_MI_CMD_MI ("stack-list-variables", mi_cmd_stack_list_variables),
+  DEF_MI_CMD_MI ("stack-select-frame", mi_cmd_stack_select_frame),
+  DEF_MI_CMD_MI ("symbol-list-lines", mi_cmd_symbol_list_lines),
+  DEF_MI_CMD_CLI ("target-attach", "attach", 1),
+  DEF_MI_CMD_MI ("target-detach", mi_cmd_target_detach),
+  DEF_MI_CMD_CLI ("target-disconnect", "disconnect", 0),
+  DEF_MI_CMD_CLI ("target-download", "load", 1),
+  DEF_MI_CMD_MI ("target-file-delete", mi_cmd_target_file_delete),
+  DEF_MI_CMD_MI ("target-file-get", mi_cmd_target_file_get),
+  DEF_MI_CMD_MI ("target-file-put", mi_cmd_target_file_put),
+  DEF_MI_CMD_CLI ("target-select", "target", 1),
+  DEF_MI_CMD_MI ("thread-info", mi_cmd_thread_info),
+  DEF_MI_CMD_MI ("thread-list-ids", mi_cmd_thread_list_ids),
+  DEF_MI_CMD_MI ("thread-select", mi_cmd_thread_select),
+  DEF_MI_CMD_MI ("trace-define-variable", mi_cmd_trace_define_variable),
+  DEF_MI_CMD_MI ("trace-find", mi_cmd_trace_find),
+  DEF_MI_CMD_MI ("trace-list-variables", mi_cmd_trace_list_variables),
+  DEF_MI_CMD_MI ("trace-save", mi_cmd_trace_save),
+  DEF_MI_CMD_MI ("trace-start", mi_cmd_trace_start),
+  DEF_MI_CMD_MI ("trace-status", mi_cmd_trace_status),
+  DEF_MI_CMD_MI ("trace-stop", mi_cmd_trace_stop),
+  DEF_MI_CMD_MI ("var-assign", mi_cmd_var_assign),
+  DEF_MI_CMD_MI ("var-create", mi_cmd_var_create),
+  DEF_MI_CMD_MI ("var-delete", mi_cmd_var_delete),
+  DEF_MI_CMD_MI ("var-evaluate-expression", mi_cmd_var_evaluate_expression),
+  DEF_MI_CMD_MI ("var-info-path-expression", mi_cmd_var_info_path_expression),
+  DEF_MI_CMD_MI ("var-info-expression", mi_cmd_var_info_expression),
+  DEF_MI_CMD_MI ("var-info-num-children", mi_cmd_var_info_num_children),
+  DEF_MI_CMD_MI ("var-info-type", mi_cmd_var_info_type),
+  DEF_MI_CMD_MI ("var-list-children", mi_cmd_var_list_children),
+  DEF_MI_CMD_MI ("var-set-format", mi_cmd_var_set_format),
+  DEF_MI_CMD_MI ("var-set-frozen", mi_cmd_var_set_frozen),
+  DEF_MI_CMD_MI ("var-set-update-range", mi_cmd_var_set_update_range),
+  DEF_MI_CMD_MI ("var-set-visualizer", mi_cmd_var_set_visualizer),
+  DEF_MI_CMD_MI ("var-show-attributes", mi_cmd_var_show_attributes),
+  DEF_MI_CMD_MI ("var-show-format", mi_cmd_var_show_format),
+  DEF_MI_CMD_MI ("var-update", mi_cmd_var_update),
   { NULL, }
 };
 
-- 
1.7.7.6

[PATCH 3/3] suppress notification

[Yao Qi]

Hi,
This patch is to change the existing 'suppress mechanism' to avoid
adding endless command name comparisons to suppress corresponding MI
notification.  This patch adds a new field 'called' in 'struct
mi_cmd', so that each MI command has a 'suppressed' flag associated to
it, and we don't have to compare command name anymore.

gdb:

2012-08-27  Yao Qi  <yao@codesourcery.com>

	* mi/mi-cmds.c (mi_cmds): New macro DEF_MI_CMD_CLI_1
	and DEF_MI_CMD_CLI_1.  Update some commands.
	* mi/mi-cmds.h (struct mi_cmd) <called>: New field.
	* mi/mi-main.c (mi_cmd_execute): Set '*parse->cmd->called' to 1.
---
 gdb/mi/mi-cmds.c |   39 +++++++++++++++++++++++++++------------
 gdb/mi/mi-cmds.h |    3 +++
 gdb/mi/mi-main.c |   13 +++----------
 3 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 0f0d74c..008f8cc 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -23,6 +23,7 @@
 #include "top.h"
 #include "mi-cmds.h"
 #include "gdb_string.h"
+#include "mi-main.h"
 
 extern void _initialize_mi_cmds (void);
 
@@ -34,25 +35,38 @@ static struct mi_cmd mi_cmds[] =
 {
 /* Define a MI command of NAME, and its corresponding CLI command is
    CLI_NAME.  */
+#define DEF_MI_CMD_CLI_1(NAME, CLI_NAME, ARGS_P, CALLED)	\
+  { NAME, { CLI_NAME, ARGS_P}, NULL, CALLED }
 #define DEF_MI_CMD_CLI(NAME, CLI_NAME, ARGS_P) \
-  { NAME, { CLI_NAME, ARGS_P}, NULL}
+  DEF_MI_CMD_CLI_1(NAME, CLI_NAME, ARGS_P, NULL)
 
 /* Define a MI command of NAME, and implemented by function MI_FUNC.  */
-#define DEF_MI_CMD_MI(NAME, MI_FUNC) { NAME, {NULL, 0}, MI_FUNC }
+#define DEF_MI_CMD_MI_1(NAME, MI_FUNC, CALLED) \
+  { NAME, {NULL, 0}, MI_FUNC, CALLED }
+#define DEF_MI_CMD_MI(NAME, MI_FUNC) DEF_MI_CMD_MI_1(NAME, MI_FUNC, NULL)
 
   DEF_MI_CMD_MI ("ada-task-info", mi_cmd_ada_task_info),
   DEF_MI_CMD_MI ("add-inferior", mi_cmd_add_inferior),
-  DEF_MI_CMD_CLI ("break-after", "ignore", 1),
-  DEF_MI_CMD_CLI ("break-condition","cond", 1),
-  DEF_MI_CMD_MI ("break-commands", mi_cmd_break_commands),
-  DEF_MI_CMD_CLI ("break-delete", "delete breakpoint", 1),
-  DEF_MI_CMD_CLI ("break-disable", "disable breakpoint", 1),
-  DEF_MI_CMD_CLI ("break-enable", "enable breakpoint", 1),
+  DEF_MI_CMD_CLI_1 ("break-after", "ignore", 1,
+		    &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_CLI_1 ("break-condition","cond", 1,
+		  &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_MI_1 ("break-commands", mi_cmd_break_commands,
+		   &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_CLI_1 ("break-delete", "delete breakpoint", 1,
+		    &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_CLI_1 ("break-disable", "disable breakpoint", 1,
+		    &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_CLI_1 ("break-enable", "enable breakpoint", 1,
+		     &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_CLI ("break-info", "info break", 1),
-  DEF_MI_CMD_MI ("break-insert", mi_cmd_break_insert),
+  DEF_MI_CMD_MI_1 ("break-insert", mi_cmd_break_insert,
+		   &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_CLI ("break-list", "info break", 0),
-  DEF_MI_CMD_MI ("break-passcount", mi_cmd_break_passcount),
-  DEF_MI_CMD_MI ("break-watch", mi_cmd_break_watch),
+  DEF_MI_CMD_MI_1 ("break-passcount", mi_cmd_break_passcount,
+		   &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_MI_1 ("break-watch", mi_cmd_break_watch,
+		   &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_MI ("data-disassemble", mi_cmd_disassemble),
   DEF_MI_CMD_MI ("data-evaluate-expression", mi_cmd_data_evaluate_expression),
   DEF_MI_CMD_MI ("data-list-changed-registers",
@@ -91,7 +105,8 @@ static struct mi_cmd mi_cmds[] =
 		 mi_cmd_file_list_exec_source_files),
   DEF_MI_CMD_CLI ("file-symbol-file", "symbol-file", 1),
   DEF_MI_CMD_MI ("gdb-exit", mi_cmd_gdb_exit),
-  DEF_MI_CMD_CLI ("gdb-set", "set", 1),
+  DEF_MI_CMD_CLI_1 ("gdb-set", "set", 1,
+		    &mi_suppress_notification.cmd_param_changed),
   DEF_MI_CMD_CLI ("gdb-show", "show", 1),
   DEF_MI_CMD_CLI ("gdb-version", "show version", 0),
   DEF_MI_CMD_MI ("inferior-tty-set", mi_cmd_inferior_tty_set),
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 4d0fc9d..3cfa8ec 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -138,6 +138,9 @@ struct mi_cmd
   struct mi_cli cli;
   /* If non-null, the function implementing the MI command.  */
   mi_cmd_argv_ftype *argv_func;
+  /* If non-null, the pointer to a flag indicates that this function is being
+     called.  */
+  int *called;
 };
 
 /* Lookup a command in the MI command table.  */
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 4db3652..554ecda 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2097,17 +2097,10 @@ mi_cmd_execute (struct mi_parse *parse)
 
   current_context = parse;
 
-  if (strncmp (parse->command, "break-", sizeof ("break-") - 1 ) == 0)
+  if (parse->cmd->called != NULL)
     {
-      make_cleanup_restore_integer (&mi_suppress_notification.breakpoint);
-      mi_suppress_notification.breakpoint = 1;
-    }
-  else if (strncmp (parse->command, "gdb-set", sizeof ("gdb-set") - 1) == 0)
-    {
-      int *p = &mi_suppress_notification.cmd_param_changed;
-
-      make_cleanup_restore_integer (p);
-      mi_suppress_notification.cmd_param_changed = 1;
+      make_cleanup_restore_integer (parse->cmd->called);
+      *parse->cmd->called = 1;
     }
 
   if (parse->cmd->argv_func != NULL)
-- 
1.7.7.6

Re: [PATCH 2/3] new macro DEF_MI_CMD_CLI and DEF_MI_CMD_MI.

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 2012-08-27  Yao Qi  <yao@codesourcery.com>
Yao> 	* mi/mi-cmds.c (mi_cmds): New macros DEF_MI_CMD_CLI and
Yao> 	DEF_MI_CMD_MI.  Adjust it.

Ok.

Tom

Re: [PATCH 3/3] suppress notification

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 2012-08-27  Yao Qi  <yao@codesourcery.com>
Yao> 	* mi/mi-cmds.c (mi_cmds): New macro DEF_MI_CMD_CLI_1
Yao> 	and DEF_MI_CMD_CLI_1.  Update some commands.
Yao> 	* mi/mi-cmds.h (struct mi_cmd) <called>: New field.
Yao> 	* mi/mi-main.c (mi_cmd_execute): Set '*parse->cmd->called' to 1.

Ok.  I like this, thanks for doing it.

Tom

Re: [PATCH 3/3] suppress notification

[Vladimir Prus]

On 27.08.2012 13:45, Yao Qi wrote:
> Hi,
> This patch is to change the existing 'suppress mechanism' to avoid
> adding endless command name comparisons to suppress corresponding MI
> notification.  This patch adds a new field 'called' in 'struct
> mi_cmd', so that each MI command has a 'suppressed' flag associated to
> it, and we don't have to compare command name anymore.

Hi Yao,

I am not 100% sure this is ideal approach. Your comment say:

> +  /* If non-null, the pointer to a flag indicates that this function is being
> +     called.  */
> +  int *called;

But in practice, this is pointer that points to notification that must be supressed when this
command is running. So, at least the comment is misleading. And if some other code will
want to check whether the current command is A, it would have to look at notification
flags.

So, at the very least, this field should have a different name, I think.

Thanks,
Volodya

Re: [PATCH 3/3] suppress notification

[Tom Tromey]

Volodya> Hi Yao,
Volodya> I am not 100% sure this is ideal approach. Your comment say:

Sorry, I didn't realize you were still reviewing MI patches.  I thought
the last time around you said you weren't.  I wouldn't have approved
this if I had known.  Next time I'll wait for your mail.

Tom

Re: [PATCH 3/3] suppress notification

[Vladimir Prus]

On 28.08.2012 06:06, Tom Tromey wrote:
> Volodya> Hi Yao,
> Volodya> I am not 100% sure this is ideal approach. Your comment say:
>
> Sorry, I didn't realize you were still reviewing MI patches.  I thought
> the last time around you said you weren't.  I wouldn't have approved
> this if I had known.  Next time I'll wait for your mail.

Hi Tom,

no problem either way - I appreciate your stepping in when you I am MIA, it's just for
this patch I had something to say and completed a response without noticing you've responded
as well.

- Volodya

Re: [PATCH 3/3] suppress notification

[Yao Qi]

On 08/28/2012 05:00 AM, Vladimir Prus wrote:
>> +  /* If non-null, the pointer to a flag indicates that this function
>> is being
>> +     called.  */
>> +  int *called;
>
> But in practice, this is pointer that points to notification that must
> be supressed when this
> command is running. So, at least the comment is misleading. And if some
> other code will
> want to check whether the current command is A, it would have to look at
> notification
> flags.
>

Although field 'called' is added for notification suppressing, but I 
don't couple this field to notification suppressing.  Ideally, field 
'called' is set to 1 when the command/function is called, as comment 
says, and set back to 0 when it is done.  At this point, it has nothing 
to do with notification suppressing, and we use this field to do 
something else in a free way.

When we want to suppress notification, we make use of the feature of 
field 'called'.   I am not sure it is misleading.

If you still think it is misleading, I'd like to rename variable 
'mi_suppress_notification' to 'mi_cmd_called'.  WDYT?

-- 
Yao

Re: [PATCH 3/3] suppress notification

[Vladimir Prus]

On 28.08.2012 11:58, Yao Qi wrote:
> On 08/28/2012 05:00 AM, Vladimir Prus wrote:
>>> +  /* If non-null, the pointer to a flag indicates that this function
>>> is being
>>> +     called.  */
>>> +  int *called;
>>
>> But in practice, this is pointer that points to notification that must
>> be supressed when this
>> command is running. So, at least the comment is misleading. And if some
>> other code will
>> want to check whether the current command is A, it would have to look at
>> notification
>> flags.
>>
>
> Although field 'called' is added for notification suppressing, but I don't couple this field to notification suppressing.  Ideally, field
> 'called' is set to 1 when the command/function is called, as comment says, and set back to 0 when it is done.  At this point, it has nothing
> to do with notification suppressing, and we use this field to do something else in a free way.
>
> When we want to suppress notification, we make use of the feature of field 'called'.   I am not sure it is misleading.

Well, the problem is that this is not a generic mechanism to everybody to know whether command X is presently running -- because
this mechanism can set only one variable, and for some commands that variable is already notification flag.

> If you still think it is misleading, I'd like to rename variable 'mi_suppress_notification' to 'mi_cmd_called'.  WDYT?

Would that be any better than just storing the name of current command and check it with strcmp? Yeah, we're back to where
we've started. What is the problem we're trying to solve? That strcmp is ugly to type and not entirely efficient?

Thanks,
Volodya

Re: [PATCH 3/3] suppress notification

[Yao Qi]

On 08/28/2012 07:56 PM, Vladimir Prus wrote:
> Well, the problem is that this is not a generic mechanism to everybody
> to know whether command X is presently running -- because
> this mechanism can set only one variable, and for some commands that
> variable is already notification flag.
>

If we want to know whether command X is running, we can add more fields 
in 'struct mi_suppress_notification', and each field is associated with 
one command in this way (set 'called' point to the address of field in 
'struct mi_suppress_notificatin').  It is unnecessary according to 
current requirement, and it can evolve easily once we have such 
requirement in the future.

>> If you still think it is misleading, I'd like to rename variable
>> 'mi_suppress_notification' to 'mi_cmd_called'.  WDYT?
>
> Would that be any better than just storing the name of current command
> and check it with strcmp? Yeah, we're back to where
> we've started. What is the problem we're trying to solve? That strcmp is
> ugly to type and not entirely efficient?

I am adding some MI notifications, which should be suppressed.  The 
problem I have is that we'll have a very long 'if/else if/else if/.../' 
blocks to compare command name to determine which suppress flag to set. 
  The code smell is not good to me.  So I draft these patches to change it.

Ideally, we can do this in a more-OO'ed manner,

   1 add a new field 'int called' in 'struct mi_cmd',
   2 set 'parse->cmd->called' in mi_cmd_execute to 1 and set it back to 
0 when it is done.
   3 pass 'struct mi_cmd *' to each MI command function, for example 
change function mi_breakpoint_created to

   mi_breakpoint_created (struct mi_cmd *self, struct breakpoint *b)

   4 inside each MI command function, return early if self->called is 1. 
  Then, we can get rid of mi_suppress_notification completely.

This will lead to more changes, so I don't implement it.  If it is 
acceptable to you, I can go to this way.

-- 
Yao

Re: [PATCH 3/3] suppress notification

[Pedro Alves]

On 08/28/2012 02:09 PM, Yao Qi wrote:
> On 08/28/2012 07:56 PM, Vladimir Prus wrote:
>> Well, the problem is that this is not a generic mechanism to everybody
>> to know whether command X is presently running -- because
>> this mechanism can set only one variable, and for some commands that
>> variable is already notification flag.
>>
> 
> If we want to know whether command X is running, we can add more fields in 'struct mi_suppress_notification', and each field is associated with one command in this way (set 'called' point to the address of field in 'struct mi_suppress_notificatin').  It is unnecessary according to current requirement, and it can evolve easily once we have such requirement in the future.
> 
>>> If you still think it is misleading, I'd like to rename variable
>>> 'mi_suppress_notification' to 'mi_cmd_called'.  WDYT?
>>
>> Would that be any better than just storing the name of current command
>> and check it with strcmp? Yeah, we're back to where
>> we've started. What is the problem we're trying to solve? That strcmp is
>> ugly to type and not entirely efficient?
> 
> I am adding some MI notifications, which should be suppressed.  The problem I have is that we'll have a very long 'if/else if/else if/.../' blocks to compare command name to determine which suppress flag to set.  The code smell is not good to me.  So I draft these patches to change it.


Alternatively, set the notification suppression down in the command callback itself.
I mention it for completeness.  Maybe you've considered it, and decided against it.

> 
> Ideally, we can do this in a more-OO'ed manner,
> 
>   1 add a new field 'int called' in 'struct mi_cmd',
>   2 set 'parse->cmd->called' in mi_cmd_execute to 1 and set it back to 0 when it is done.
>   3 pass 'struct mi_cmd *' to each MI command function, for example change function mi_breakpoint_created to
> 
>   mi_breakpoint_created (struct mi_cmd *self, struct breakpoint *b)
> 
>   4 inside each MI command function, return early if self->called is 1.  Then, we can get rid of mi_suppress_notification completely.

Confused.  mi_breakpoint_created is not a MI command function, but rather a
notification observer.  Whoever calls the observers (observer_notify_breakpoint_created)
is disconnected from commands, and I don't see that the coupling would be a good idea.

> 
> This will lead to more changes, so I don't implement it.  If it is acceptable to you, I can go to this way.
> 

-- 
Pedro Alves

Re: [PATCH 3/3] suppress notification

[Yao Qi]

On 08/28/2012 09:39 PM, Pedro Alves wrote:
>
> Alternatively, set the notification suppression down in the command callback itself.
> I mention it for completeness.  Maybe you've considered it, and decided against it.
>

Are you suggesting that we can set breakpoint suppression flag in each 
breakpoint related CLI commands, such as "delete breakpoint", "disable 
breakpoint", and so forth?

>> >
>> >Ideally, we can do this in a more-OO'ed manner,
>> >
>> >   1 add a new field 'int called' in 'struct mi_cmd',
>> >   2 set 'parse->cmd->called' in mi_cmd_execute to 1 and set it back to 0 when it is done.
>> >   3 pass 'struct mi_cmd *' to each MI command function, for example change function mi_breakpoint_created to
>> >
>> >   mi_breakpoint_created (struct mi_cmd *self, struct breakpoint *b)
>> >
>> >   4 inside each MI command function, return early if self->called is 1.  Then, we can get rid of mi_suppress_notification completely.
> Confused.  mi_breakpoint_created is not a MI command function, but rather a
> notification observer.  Whoever calls the observers (observer_notify_breakpoint_created)
> is disconnected from commands, and I don't see that the coupling would be a good idea.
>

Oops, my brain stopped working at that moment.  Sorry for the confusion, 
and please ignore it.

-- 
Yao

Re: [PATCH 3/3] suppress notification

[Pedro Alves]

On 08/28/2012 02:49 PM, Yao Qi wrote:
> On 08/28/2012 09:39 PM, Pedro Alves wrote:
>>
>> Alternatively, set the notification suppression down in the command callback itself.
>> I mention it for completeness.  Maybe you've considered it, and decided against it.
>>
> 
> Are you suggesting that we can set breakpoint suppression flag in each breakpoint related CLI commands, such as "delete breakpoint", "disable breakpoint", and so forth?

Not really a strong suggestion, but more like it seemed like the obvious
first choice to me, so I wondered if you considered and discarded it.
You wouldn't do it within the CLI commands.  Instead you'd expand:

  { "break-delete", { "delete breakpoint", 1 }, NULL },

into something like

  { "break-delete", { NULL, 0 }, mi_cmd_break_delete },

and then mi_cmd_break_delete would suppress the notification and
call "delete breakpoint" itself.  Or a core API function to break
the CLI dependency (which might be a good idea anyway).

-- 
Pedro Alves

Re: [PATCH 3/3] suppress notification

[Yao Qi]

On 08/28/2012 05:00 AM, Vladimir Prus wrote:
>> +  /* If non-null, the pointer to a flag indicates that this function 
>> is being
>> +     called.  */
>> +  int *called;
> 
> But in practice, this is pointer that points to notification that must 
> be supressed when this
> command is running. So, at least the comment is misleading. And if some 
> other code will
> want to check whether the current command is A, it would have to look at 
> notification
> flags.
> 
> So, at the very least, this field should have a different name, I think.
Vladimir,
The field name is changed to 'suppress_notification' with some comments
update.  How about this one?

-- 
Yao

gdb:

2012-08-31  Yao Qi  <yao@codesourcery.com>

	* mi/mi-cmds.c (mi_cmds): Include 'mi-main.h'.
	New macro DEF_MI_CMD_CLI_1 and DEF_MI_CMD_CLI_1.  Update some
	commands.
	* mi/mi-cmds.h (struct mi_cmd) <suppress_notification>: New field.
	* mi/mi-main.c (mi_cmd_execute): Set '*parse->cmd->suppress_notification'
	to 1.
---
 gdb/mi/mi-cmds.c |   39 +++++++++++++++++++++++++++------------
 gdb/mi/mi-cmds.h |    6 ++++++
 gdb/mi/mi-main.c |   13 +++----------
 3 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 0f0d74c..008f8cc 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -23,6 +23,7 @@
 #include "top.h"
 #include "mi-cmds.h"
 #include "gdb_string.h"
+#include "mi-main.h"
 
 extern void _initialize_mi_cmds (void);
 
@@ -34,25 +35,38 @@ static struct mi_cmd mi_cmds[] =
 {
 /* Define a MI command of NAME, and its corresponding CLI command is
    CLI_NAME.  */
+#define DEF_MI_CMD_CLI_1(NAME, CLI_NAME, ARGS_P, CALLED)	\
+  { NAME, { CLI_NAME, ARGS_P}, NULL, CALLED }
 #define DEF_MI_CMD_CLI(NAME, CLI_NAME, ARGS_P) \
-  { NAME, { CLI_NAME, ARGS_P}, NULL}
+  DEF_MI_CMD_CLI_1(NAME, CLI_NAME, ARGS_P, NULL)
 
 /* Define a MI command of NAME, and implemented by function MI_FUNC.  */
-#define DEF_MI_CMD_MI(NAME, MI_FUNC) { NAME, {NULL, 0}, MI_FUNC }
+#define DEF_MI_CMD_MI_1(NAME, MI_FUNC, CALLED) \
+  { NAME, {NULL, 0}, MI_FUNC, CALLED }
+#define DEF_MI_CMD_MI(NAME, MI_FUNC) DEF_MI_CMD_MI_1(NAME, MI_FUNC, NULL)
 
   DEF_MI_CMD_MI ("ada-task-info", mi_cmd_ada_task_info),
   DEF_MI_CMD_MI ("add-inferior", mi_cmd_add_inferior),
-  DEF_MI_CMD_CLI ("break-after", "ignore", 1),
-  DEF_MI_CMD_CLI ("break-condition","cond", 1),
-  DEF_MI_CMD_MI ("break-commands", mi_cmd_break_commands),
-  DEF_MI_CMD_CLI ("break-delete", "delete breakpoint", 1),
-  DEF_MI_CMD_CLI ("break-disable", "disable breakpoint", 1),
-  DEF_MI_CMD_CLI ("break-enable", "enable breakpoint", 1),
+  DEF_MI_CMD_CLI_1 ("break-after", "ignore", 1,
+		    &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_CLI_1 ("break-condition","cond", 1,
+		  &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_MI_1 ("break-commands", mi_cmd_break_commands,
+		   &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_CLI_1 ("break-delete", "delete breakpoint", 1,
+		    &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_CLI_1 ("break-disable", "disable breakpoint", 1,
+		    &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_CLI_1 ("break-enable", "enable breakpoint", 1,
+		     &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_CLI ("break-info", "info break", 1),
-  DEF_MI_CMD_MI ("break-insert", mi_cmd_break_insert),
+  DEF_MI_CMD_MI_1 ("break-insert", mi_cmd_break_insert,
+		   &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_CLI ("break-list", "info break", 0),
-  DEF_MI_CMD_MI ("break-passcount", mi_cmd_break_passcount),
-  DEF_MI_CMD_MI ("break-watch", mi_cmd_break_watch),
+  DEF_MI_CMD_MI_1 ("break-passcount", mi_cmd_break_passcount,
+		   &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_MI_1 ("break-watch", mi_cmd_break_watch,
+		   &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_MI ("data-disassemble", mi_cmd_disassemble),
   DEF_MI_CMD_MI ("data-evaluate-expression", mi_cmd_data_evaluate_expression),
   DEF_MI_CMD_MI ("data-list-changed-registers",
@@ -91,7 +105,8 @@ static struct mi_cmd mi_cmds[] =
 		 mi_cmd_file_list_exec_source_files),
   DEF_MI_CMD_CLI ("file-symbol-file", "symbol-file", 1),
   DEF_MI_CMD_MI ("gdb-exit", mi_cmd_gdb_exit),
-  DEF_MI_CMD_CLI ("gdb-set", "set", 1),
+  DEF_MI_CMD_CLI_1 ("gdb-set", "set", 1,
+		    &mi_suppress_notification.cmd_param_changed),
   DEF_MI_CMD_CLI ("gdb-show", "show", 1),
   DEF_MI_CMD_CLI ("gdb-version", "show version", 0),
   DEF_MI_CMD_MI ("inferior-tty-set", mi_cmd_inferior_tty_set),
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 4d0fc9d..cf1a5eb 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -138,6 +138,12 @@ struct mi_cmd
   struct mi_cli cli;
   /* If non-null, the function implementing the MI command.  */
   mi_cmd_argv_ftype *argv_func;
+  /* If non-null, the pointer to a field in
+     'struct mi_suppress_notification', which will be set to true by MI
+     command processor (mi-main.c:mi_cmd_execute) when this command is
+     being executed.  It will be set back to false when command has been
+     executed.  */
+  int *suppress_notification;
 };
 
 /* Lookup a command in the MI command table.  */
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 4db3652..f1d21bc 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2097,17 +2097,10 @@ mi_cmd_execute (struct mi_parse *parse)
 
   current_context = parse;
 
-  if (strncmp (parse->command, "break-", sizeof ("break-") - 1 ) == 0)
+  if (parse->cmd->suppress_notification != NULL)
     {
-      make_cleanup_restore_integer (&mi_suppress_notification.breakpoint);
-      mi_suppress_notification.breakpoint = 1;
-    }
-  else if (strncmp (parse->command, "gdb-set", sizeof ("gdb-set") - 1) == 0)
-    {
-      int *p = &mi_suppress_notification.cmd_param_changed;
-
-      make_cleanup_restore_integer (p);
-      mi_suppress_notification.cmd_param_changed = 1;
+      make_cleanup_restore_integer (parse->cmd->suppress_notification);
+      *parse->cmd->suppress_notification = 1;
     }
 
   if (parse->cmd->argv_func != NULL)
-- 
1.7.7.6

Re: [PATCH 3/3] suppress notification

[Vladimir Prus]

On 31.08.2012 12:07, Yao Qi wrote:
> On 08/28/2012 05:00 AM, Vladimir Prus wrote:
>>> +  /* If non-null, the pointer to a flag indicates that this function
>>> is being
>>> +     called.  */
>>> +  int *called;
>>
>> But in practice, this is pointer that points to notification that must
>> be supressed when this
>> command is running. So, at least the comment is misleading. And if some
>> other code will
>> want to check whether the current command is A, it would have to look at
>> notification
>> flags.
>>
>> So, at the very least, this field should have a different name, I think.
> Vladimir,
> The field name is changed to 'suppress_notification' with some comments
> update.  How about this one?

Yao,

I have no objections to this version.

Thanks,
Volodya

[committed]: [PATCH 3/3] suppress notification

[Yao Qi]

On 08/31/2012 04:22 PM, Vladimir Prus wrote:
> I have no objections to this version.

Thanks the review.  I've committed them.

-- 
Yao