* [PATCH 0/3] Factor code on suppress MI notification
@ 2012-08-27 9:46 Yao Qi
2012-08-27 9:46 ` [PATCH 2/3] new macro DEF_MI_CMD_CLI and DEF_MI_CMD_MI Yao Qi
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Yao Qi @ 2012-08-27 9:46 UTC (permalink / raw)
To: gdb-patches
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?
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 2/3] new macro DEF_MI_CMD_CLI and DEF_MI_CMD_MI. 2012-08-27 9:46 [PATCH 0/3] Factor code on suppress MI notification Yao Qi @ 2012-08-27 9:46 ` Yao Qi 2012-08-27 20:18 ` Tom Tromey 2012-08-27 9:46 ` [PATCH 3/3] suppress notification Yao Qi 2012-08-27 9:46 ` [PATCH 1/3] add static to mi_cmds Yao Qi 2 siblings, 1 reply; 18+ messages in thread From: Yao Qi @ 2012-08-27 9:46 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] new macro DEF_MI_CMD_CLI and DEF_MI_CMD_MI. 2012-08-27 9:46 ` [PATCH 2/3] new macro DEF_MI_CMD_CLI and DEF_MI_CMD_MI Yao Qi @ 2012-08-27 20:18 ` Tom Tromey 0 siblings, 0 replies; 18+ messages in thread From: Tom Tromey @ 2012-08-27 20:18 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches >>>>> "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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] suppress notification 2012-08-27 9:46 [PATCH 0/3] Factor code on suppress MI notification Yao Qi 2012-08-27 9:46 ` [PATCH 2/3] new macro DEF_MI_CMD_CLI and DEF_MI_CMD_MI Yao Qi @ 2012-08-27 9:46 ` Yao Qi 2012-08-27 20:20 ` Tom Tromey 2012-08-27 21:01 ` Vladimir Prus 2012-08-27 9:46 ` [PATCH 1/3] add static to mi_cmds Yao Qi 2 siblings, 2 replies; 18+ messages in thread From: Yao Qi @ 2012-08-27 9:46 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-27 9:46 ` [PATCH 3/3] suppress notification Yao Qi @ 2012-08-27 20:20 ` Tom Tromey 2012-08-27 21:01 ` Vladimir Prus 1 sibling, 0 replies; 18+ messages in thread From: Tom Tromey @ 2012-08-27 20:20 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches >>>>> "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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-27 9:46 ` [PATCH 3/3] suppress notification Yao Qi 2012-08-27 20:20 ` Tom Tromey @ 2012-08-27 21:01 ` Vladimir Prus 2012-08-28 2:06 ` Tom Tromey ` (2 more replies) 1 sibling, 3 replies; 18+ messages in thread From: Vladimir Prus @ 2012-08-27 21:01 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-27 21:01 ` Vladimir Prus @ 2012-08-28 2:06 ` Tom Tromey 2012-08-28 4:50 ` Vladimir Prus 2012-08-28 7:58 ` Yao Qi 2012-08-31 8:07 ` Yao Qi 2 siblings, 1 reply; 18+ messages in thread From: Tom Tromey @ 2012-08-28 2:06 UTC (permalink / raw) To: Vladimir Prus; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-28 2:06 ` Tom Tromey @ 2012-08-28 4:50 ` Vladimir Prus 0 siblings, 0 replies; 18+ messages in thread From: Vladimir Prus @ 2012-08-28 4:50 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-27 21:01 ` Vladimir Prus 2012-08-28 2:06 ` Tom Tromey @ 2012-08-28 7:58 ` Yao Qi 2012-08-28 11:57 ` Vladimir Prus 2012-08-31 8:07 ` Yao Qi 2 siblings, 1 reply; 18+ messages in thread From: Yao Qi @ 2012-08-28 7:58 UTC (permalink / raw) To: Vladimir Prus; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-28 7:58 ` Yao Qi @ 2012-08-28 11:57 ` Vladimir Prus 2012-08-28 13:09 ` Yao Qi 0 siblings, 1 reply; 18+ messages in thread From: Vladimir Prus @ 2012-08-28 11:57 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-28 11:57 ` Vladimir Prus @ 2012-08-28 13:09 ` Yao Qi 2012-08-28 13:40 ` Pedro Alves 0 siblings, 1 reply; 18+ messages in thread From: Yao Qi @ 2012-08-28 13:09 UTC (permalink / raw) To: Vladimir Prus; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-28 13:09 ` Yao Qi @ 2012-08-28 13:40 ` Pedro Alves 2012-08-28 13:50 ` Yao Qi 0 siblings, 1 reply; 18+ messages in thread From: Pedro Alves @ 2012-08-28 13:40 UTC (permalink / raw) To: Yao Qi; +Cc: Vladimir Prus, gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-28 13:40 ` Pedro Alves @ 2012-08-28 13:50 ` Yao Qi 2012-08-28 14:09 ` Pedro Alves 0 siblings, 1 reply; 18+ messages in thread From: Yao Qi @ 2012-08-28 13:50 UTC (permalink / raw) To: Pedro Alves; +Cc: Vladimir Prus, gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-28 13:50 ` Yao Qi @ 2012-08-28 14:09 ` Pedro Alves 0 siblings, 0 replies; 18+ messages in thread From: Pedro Alves @ 2012-08-28 14:09 UTC (permalink / raw) To: Yao Qi; +Cc: Vladimir Prus, gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-27 21:01 ` Vladimir Prus 2012-08-28 2:06 ` Tom Tromey 2012-08-28 7:58 ` Yao Qi @ 2012-08-31 8:07 ` Yao Qi 2012-08-31 8:22 ` Vladimir Prus 2 siblings, 1 reply; 18+ messages in thread From: Yao Qi @ 2012-08-31 8:07 UTC (permalink / raw) To: Vladimir Prus; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] suppress notification 2012-08-31 8:07 ` Yao Qi @ 2012-08-31 8:22 ` Vladimir Prus 2012-08-31 8:49 ` [committed]: " Yao Qi 0 siblings, 1 reply; 18+ messages in thread From: Vladimir Prus @ 2012-08-31 8:22 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [committed]: [PATCH 3/3] suppress notification 2012-08-31 8:22 ` Vladimir Prus @ 2012-08-31 8:49 ` Yao Qi 0 siblings, 0 replies; 18+ messages in thread From: Yao Qi @ 2012-08-31 8:49 UTC (permalink / raw) Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] add static to mi_cmds 2012-08-27 9:46 [PATCH 0/3] Factor code on suppress MI notification Yao Qi 2012-08-27 9:46 ` [PATCH 2/3] new macro DEF_MI_CMD_CLI and DEF_MI_CMD_MI Yao Qi 2012-08-27 9:46 ` [PATCH 3/3] suppress notification Yao Qi @ 2012-08-27 9:46 ` Yao Qi 2 siblings, 0 replies; 18+ messages in thread From: Yao Qi @ 2012-08-27 9:46 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2012-08-31 8:49 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-08-27 9:46 [PATCH 0/3] Factor code on suppress MI notification Yao Qi 2012-08-27 9:46 ` [PATCH 2/3] new macro DEF_MI_CMD_CLI and DEF_MI_CMD_MI Yao Qi 2012-08-27 20:18 ` Tom Tromey 2012-08-27 9:46 ` [PATCH 3/3] suppress notification Yao Qi 2012-08-27 20:20 ` Tom Tromey 2012-08-27 21:01 ` Vladimir Prus 2012-08-28 2:06 ` Tom Tromey 2012-08-28 4:50 ` Vladimir Prus 2012-08-28 7:58 ` Yao Qi 2012-08-28 11:57 ` Vladimir Prus 2012-08-28 13:09 ` Yao Qi 2012-08-28 13:40 ` Pedro Alves 2012-08-28 13:50 ` Yao Qi 2012-08-28 14:09 ` Pedro Alves 2012-08-31 8:07 ` Yao Qi 2012-08-31 8:22 ` Vladimir Prus 2012-08-31 8:49 ` [committed]: " Yao Qi 2012-08-27 9:46 ` [PATCH 1/3] add static to mi_cmds Yao Qi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox