Re: [PATCH 3/3] suppress notification

[Yao Qi <yao@codesourcery.com>]

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