Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 2/6] attach to command_option-changed observer.
  2012-07-27 15:23 [RCF 0/6 V2] MI notification of command option change Yao Qi
  2012-07-27 15:23 ` [PATCH 3/6] notify in enum_cmd and scheduler-locking Yao Qi
  2012-07-27 15:23 ` [PATCH 6/6] code indentation Yao Qi
@ 2012-07-27 15:23 ` Yao Qi
  2012-07-27 17:20   ` Tom Tromey
  2012-07-27 17:44   ` Tom Tromey
  2012-07-27 15:23 ` [PATCH 1/6] new observer command_option_changed Yao Qi
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 13+ messages in thread
From: Yao Qi @ 2012-07-27 15:23 UTC (permalink / raw)
  To: gdb-patches

Hi,
'mi_suppress_breakpoint_notifications' is used to suppress mi
breakpoint notifications.  This patch is to change this variable
to an array so that multiple mi notifications can be suppressed.
The following patch will use it to suppress other MI notifications.

This patch is to attach function 'mi_command_option_changed' to
observer 'command_option_changed', so that a MI notification
"=option-changed" is sent to MI frontend.  If the command option
change is requested from MI, the notification is suppressed.

gdb:

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

	* NEWS: Mention new MI notification.

	* mi/mi-interp.c: Declare mi_command_option_changed.
	(mi_interpreter_init): Attach mi_command_option_changed to
	observer command_option_changed.
	(mi_command_option_changed): New.
	Remove mi_suppress_breakpoint_notifications.
	Define global variable mi_suppress_notification.
	(mi_breakpoint_created): Update.
	(mi_breakpoint_deleted): Likewise.
	(mi_breakpoint_modified): Likewise.
	* mi/mi-main.c (mi_cmd_execute): Likewise.  Check command
	'gdb-set' and set mi_suppress_notification.
	* mi/mi-main.h: (mi_suppress_notification): New struct.

gdb/doc:

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

	* gdb.texinfo (GDB/MI Async Records): Doc for '=option-changed'.
---
 gdb/NEWS            |    5 +++++
 gdb/doc/gdb.texinfo |    3 +++
 gdb/mi/mi-interp.c  |   44 ++++++++++++++++++++++++++++++++++++++------
 gdb/mi/mi-main.c    |    9 +++++++--
 gdb/mi/mi-main.h    |   10 +++++++++-
 5 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 3333810..0f73e60 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,11 @@
 
 *** Changes since GDB 7.5
 
+* MI changes
+
+  ** Command option changes are now notified using new async record
+     "=option-changed".
+
 *** Changes in GDB 7.5
 
 * GDB now supports x32 ABI.  Visit <http://sites.google.com/site/x32abi/>
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 68ea817..aa95a91 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -27638,6 +27638,9 @@ breakpoint commands; @xref{GDB/MI Breakpoint Commands}.
 Note that if a breakpoint is emitted in the result record of a
 command, then it will not also be emitted in an async record.
 
+@item =option-changed,option=@var{option},value=@var{value}
+Reports that an option of the command @code{set @var{option}} is
+changed to @var{value}.
 @end table
 
 @node GDB/MI Frame Information
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index b487136..58bbfe6 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -71,6 +71,7 @@ static void mi_about_to_proceed (void);
 static void mi_breakpoint_created (struct breakpoint *b);
 static void mi_breakpoint_deleted (struct breakpoint *b);
 static void mi_breakpoint_modified (struct breakpoint *b);
+static void mi_command_option_changed (const char *option, const char *value);
 
 static int report_initial_inferior (struct inferior *inf, void *closure);
 
@@ -128,6 +129,7 @@ mi_interpreter_init (struct interp *interp, int top_level)
       observer_attach_breakpoint_created (mi_breakpoint_created);
       observer_attach_breakpoint_deleted (mi_breakpoint_deleted);
       observer_attach_breakpoint_modified (mi_breakpoint_modified);
+      observer_attach_command_option_changed (mi_command_option_changed);
 
       /* The initial inferior is created before this function is
 	 called, so we need to report it explicitly.  Use iteration in
@@ -501,10 +503,14 @@ mi_about_to_proceed (void)
   mi_proceeded = 1;
 }
 
-/* When non-zero, no MI notifications will be emitted in
-   response to breakpoint change observers.  */
+/* When the element is non-zero, no MI notifications will be emitted in
+   response to the corresponding observers.  */
 
-int mi_suppress_breakpoint_notifications = 0;
+struct mi_suppress_notification mi_suppress_notification =
+  {
+    0,
+    0,
+  };
 
 /* Emit notification about a created breakpoint.  */
 
@@ -515,7 +521,7 @@ mi_breakpoint_created (struct breakpoint *b)
   struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
   volatile struct gdb_exception e;
 
-  if (mi_suppress_breakpoint_notifications)
+  if (mi_suppress_notification.breakpoint)
     return;
 
   if (b->number <= 0)
@@ -546,7 +552,7 @@ mi_breakpoint_deleted (struct breakpoint *b)
 {
   struct mi_interp *mi = top_level_interpreter_data ();
 
-  if (mi_suppress_breakpoint_notifications)
+  if (mi_suppress_notification.breakpoint)
     return;
 
   if (b->number <= 0)
@@ -569,7 +575,7 @@ mi_breakpoint_modified (struct breakpoint *b)
   struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
   volatile struct gdb_exception e;
 
-  if (mi_suppress_breakpoint_notifications)
+  if (mi_suppress_notification.breakpoint)
     return;
 
   if (b->number <= 0)
@@ -730,6 +736,32 @@ mi_solib_unloaded (struct so_list *solib)
   gdb_flush (mi->event_channel);
 }
 
+/* Emit notification about the command option change.  */
+
+static void
+mi_command_option_changed (const char *option, const char *value)
+{
+  struct mi_interp *mi = top_level_interpreter_data ();
+  struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
+
+  if (mi_suppress_notification.option_changed)
+    return;
+
+  target_terminal_ours ();
+
+  fprintf_unfiltered (mi->event_channel,
+		      "option-changed");
+
+  ui_out_redirect (mi_uiout, mi->event_channel);
+
+  ui_out_field_string (mi_uiout, "option", option);
+  ui_out_field_string (mi_uiout, "value", value);
+
+  ui_out_redirect (mi_uiout, NULL);
+
+  gdb_flush (mi->event_channel);
+}
+
 static int
 report_initial_inferior (struct inferior *inf, void *closure)
 {
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index dfb4892..f82b751 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2099,8 +2099,13 @@ mi_cmd_execute (struct mi_parse *parse)
 
   if (strncmp (parse->command, "break-", sizeof ("break-") - 1 ) == 0)
     {
-      make_cleanup_restore_integer (&mi_suppress_breakpoint_notifications);
-      mi_suppress_breakpoint_notifications = 1;
+      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)
+    {
+      make_cleanup_restore_integer (&mi_suppress_notification.option_changed);
+      mi_suppress_notification.option_changed = 1;
     }
 
   if (parse->cmd->argv_func != NULL)
diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h
index beac2cd..b940154 100644
--- a/gdb/mi/mi-main.h
+++ b/gdb/mi/mi-main.h
@@ -32,7 +32,15 @@ extern char *current_token;
 
 extern int running_result_record_printed;
 extern int mi_proceeded;
-extern int mi_suppress_breakpoint_notifications;
+
+struct mi_suppress_notification
+{
+  /* Breakpoint notification suppressed? */
+  int breakpoint;
+  /* Command option changed notification suppressed? */
+  int option_changed;
+};
+extern struct mi_suppress_notification mi_suppress_notification;
 
 #endif
 
-- 
1.7.7.6


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

* [PATCH 6/6] code indentation.
  2012-07-27 15:23 [RCF 0/6 V2] MI notification of command option change Yao Qi
  2012-07-27 15:23 ` [PATCH 3/6] notify in enum_cmd and scheduler-locking Yao Qi
@ 2012-07-27 15:23 ` Yao Qi
  2012-07-27 15:23 ` [PATCH 2/6] attach to command_option-changed observer Yao Qi
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2012-07-27 15:23 UTC (permalink / raw)
  To: gdb-patches

gdb:

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

	* cli/cli-setshow.c (do_set_command): Code indentation.
	(do_show_command): Likewise.
---
 gdb/cli/cli-setshow.c |  538 ++++++++++++++++++++++++-------------------------
 1 files changed, 268 insertions(+), 270 deletions(-)

diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 17c336e..7a1dd4e 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -130,247 +130,247 @@ do_set_command (char *arg, int from_tty, struct cmd_list_element *c)
 
   gdb_assert (c->type == set_cmd);
 
-      switch (c->var_type)
-	{
-	case var_string:
+  switch (c->var_type)
+    {
+    case var_string:
+      {
+	char *new;
+	char *p;
+	char *q;
+	int ch;
+
+	if (arg == NULL)
+	  arg = "";
+	new = (char *) xmalloc (strlen (arg) + 2);
+	p = arg;
+	q = new;
+	while ((ch = *p++) != '\000')
 	  {
-	    char *new;
-	    char *p;
-	    char *q;
-	    int ch;
-
-	    if (arg == NULL)
-	      arg = "";
-	    new = (char *) xmalloc (strlen (arg) + 2);
-	    p = arg;
-	    q = new;
-	    while ((ch = *p++) != '\000')
+	    if (ch == '\\')
 	      {
-		if (ch == '\\')
-		  {
-		    /* \ at end of argument is used after spaces
-		       so they won't be lost.  */
-		    /* This is obsolete now that we no longer strip
-		       trailing whitespace and actually, the backslash
-		       didn't get here in my test, readline or
-		       something did something funky with a backslash
-		       right before a newline.  */
-		    if (*p == 0)
-		      break;
-		    ch = parse_escape (get_current_arch (), &p);
-		    if (ch == 0)
-		      break;	/* C loses */
-		    else if (ch > 0)
-		      *q++ = ch;
-		  }
-		else
+		/* \ at end of argument is used after spaces
+		   so they won't be lost.  */
+		/* This is obsolete now that we no longer strip
+		   trailing whitespace and actually, the backslash
+		   didn't get here in my test, readline or
+		   something did something funky with a backslash
+		   right before a newline.  */
+		if (*p == 0)
+		  break;
+		ch = parse_escape (get_current_arch (), &p);
+		if (ch == 0)
+		  break;	/* C loses */
+		else if (ch > 0)
 		  *q++ = ch;
 	      }
+	    else
+	      *q++ = ch;
+	  }
 #if 0
-	    if (*(p - 1) != '\\')
-	      *q++ = ' ';
+	if (*(p - 1) != '\\')
+	  *q++ = ' ';
 #endif
-	    *q++ = '\0';
-	    new = (char *) xrealloc (new, q - new);
+	*q++ = '\0';
+	new = (char *) xrealloc (new, q - new);
 
-	    if (*(char **) c->var == NULL
-		|| strcmp (*(char **) c->var, new) != 0)
-	      {
-		xfree (*(char **) c->var);
-		*(char **) c->var = new;
+	if (*(char **) c->var == NULL
+	    || strcmp (*(char **) c->var, new) != 0)
+	  {
+	    xfree (*(char **) c->var);
+	    *(char **) c->var = new;
 
-		option_changed = 1;
-	      }
-	    else
-	      xfree (new);
+	    option_changed = 1;
 	  }
-	  break;
-	case var_string_noescape:
-	  if (arg == NULL)
-	    arg = "";
+	else
+	  xfree (new);
+      }
+      break;
+    case var_string_noescape:
+      if (arg == NULL)
+	arg = "";
 
-	  if (*(char **) c->var == NULL || strcmp (*(char **) c->var, arg) != 0)
-	    {
-	      xfree (*(char **) c->var);
-	      *(char **) c->var = xstrdup (arg);
+      if (*(char **) c->var == NULL || strcmp (*(char **) c->var, arg) != 0)
+	{
+	  xfree (*(char **) c->var);
+	  *(char **) c->var = xstrdup (arg);
 
-	      option_changed = 1;
-	    }
-	  break;
-	case var_filename:
-	  if (arg == NULL)
-	    error_no_arg (_("filename to set it to."));
-	  /* FALLTHROUGH */
-	case var_optional_filename:
-	  {
-	    char *val = NULL;
+	  option_changed = 1;
+	}
+      break;
+    case var_filename:
+      if (arg == NULL)
+	error_no_arg (_("filename to set it to."));
+      /* FALLTHROUGH */
+    case var_optional_filename:
+      {
+	char *val = NULL;
 
-	    if (arg != NULL)
-	      {
-		/* Clear trailing whitespace of filename.  */
-		char *ptr = arg + strlen (arg) - 1;
+	if (arg != NULL)
+	  {
+	    /* Clear trailing whitespace of filename.  */
+	    char *ptr = arg + strlen (arg) - 1;
 
-		while (ptr >= arg && (*ptr == ' ' || *ptr == '\t'))
-		  ptr--;
-		*(ptr + 1) = '\0';
+	    while (ptr >= arg && (*ptr == ' ' || *ptr == '\t'))
+	      ptr--;
+	    *(ptr + 1) = '\0';
 
-		val = tilde_expand (arg);
-	      }
-	    else
-	      val = xstrdup ("");
+	    val = tilde_expand (arg);
+	  }
+	else
+	  val = xstrdup ("");
 
-	    if (*(char **) c->var == NULL
-		|| strcmp (*(char **) c->var, val) != 0)
-	      {
-		xfree (*(char **) c->var);
-		*(char **) c->var = val;
+	if (*(char **) c->var == NULL
+	    || strcmp (*(char **) c->var, val) != 0)
+	  {
+	    xfree (*(char **) c->var);
+	    *(char **) c->var = val;
 
-		option_changed = 1;
-	      }
-	    else
-	      xfree (val);
+	    option_changed = 1;
 	  }
-	  break;
-	case var_boolean:
-	  {
-	    int val = parse_binary_operation (arg);
+	else
+	  xfree (val);
+      }
+      break;
+    case var_boolean:
+      {
+	int val = parse_binary_operation (arg);
 
-	    if (val != *(int *) c->var)
-	      {
-		*(int *) c->var = val;
+	if (val != *(int *) c->var)
+	  {
+	    *(int *) c->var = val;
 
-		option_changed = 1;
-	      }
+	    option_changed = 1;
 	  }
-	  break;
-	case var_auto_boolean:
-	  {
-	    enum auto_boolean val = parse_auto_binary_operation (arg);
+      }
+      break;
+    case var_auto_boolean:
+      {
+	enum auto_boolean val = parse_auto_binary_operation (arg);
 
-	    if (*(enum auto_boolean *) c->var != val)
-	      {
-		*(enum auto_boolean *) c->var = val;
+	if (*(enum auto_boolean *) c->var != val)
+	  {
+	    *(enum auto_boolean *) c->var = val;
 
-		option_changed = 1;
-	      }
+	    option_changed = 1;
 	  }
-	  break;
-	case var_uinteger:
-	case var_zuinteger:
-	  if (arg == NULL)
-	    error_no_arg (_("integer to set it to."));
-	  {
-	    unsigned int val = parse_and_eval_long (arg);
+      }
+      break;
+    case var_uinteger:
+    case var_zuinteger:
+      if (arg == NULL)
+	error_no_arg (_("integer to set it to."));
+      {
+	unsigned int val = parse_and_eval_long (arg);
 
-	    if (c->var_type == var_uinteger && val == 0)
-	      val = UINT_MAX;
+	if (c->var_type == var_uinteger && val == 0)
+	  val = UINT_MAX;
 
-	    if (*(unsigned int *) c->var != val)
-	      {
-		*(unsigned int *) c->var = val;
+	if (*(unsigned int *) c->var != val)
+	  {
+	    *(unsigned int *) c->var = val;
 
-		option_changed = 1;
-	      }
+	    option_changed = 1;
 	  }
-	  break;
-	case var_integer:
-	case var_zinteger:
-	  {
-	    unsigned int val;
+      }
+      break;
+    case var_integer:
+    case var_zinteger:
+      {
+	unsigned int val;
 
-	    if (arg == NULL)
-	      error_no_arg (_("integer to set it to."));
-	    val = parse_and_eval_long (arg);
-	    if (val == 0 && c->var_type == var_integer)
-	      val = INT_MAX;
-	    else if (val >= INT_MAX)
-	      error (_("integer %u out of range"), val);
+	if (arg == NULL)
+	  error_no_arg (_("integer to set it to."));
+	val = parse_and_eval_long (arg);
+	if (val == 0 && c->var_type == var_integer)
+	  val = INT_MAX;
+	else if (val >= INT_MAX)
+	  error (_("integer %u out of range"), val);
 
 
-	    if (*(int *) c->var != val)
-	      {
-		*(int *) c->var = val;
+	if (*(int *) c->var != val)
+	  {
+	    *(int *) c->var = val;
 
-		option_changed = 1;
-	      }
-	    break;
+	    option_changed = 1;
 	  }
-	case var_enum:
+	break;
+      }
+    case var_enum:
+      {
+	int i;
+	int len;
+	int nmatches;
+	const char *match = NULL;
+	char *p;
+
+	/* If no argument was supplied, print an informative error
+	   message.  */
+	if (arg == NULL)
 	  {
-	    int i;
-	    int len;
-	    int nmatches;
-	    const char *match = NULL;
-	    char *p;
-
-	    /* If no argument was supplied, print an informative error
-	       message.  */
-	    if (arg == NULL)
+	    char *msg;
+	    int msg_len = 0;
+
+	    for (i = 0; c->enums[i]; i++)
+	      msg_len += strlen (c->enums[i]) + 2;
+
+	    msg = xmalloc (msg_len);
+	    *msg = '\0';
+	    make_cleanup (xfree, msg);
+
+	    for (i = 0; c->enums[i]; i++)
 	      {
-		char *msg;
-		int msg_len = 0;
-
-		for (i = 0; c->enums[i]; i++)
-		  msg_len += strlen (c->enums[i]) + 2;
-
-		msg = xmalloc (msg_len);
-		*msg = '\0';
-		make_cleanup (xfree, msg);
-		
-		for (i = 0; c->enums[i]; i++)
-		  {
-		    if (i != 0)
-		      strcat (msg, ", ");
-		    strcat (msg, c->enums[i]);
-		  }
-		error (_("Requires an argument. Valid arguments are %s."), 
-		       msg);
+		if (i != 0)
+		  strcat (msg, ", ");
+		strcat (msg, c->enums[i]);
 	      }
+	    error (_("Requires an argument. Valid arguments are %s."), 
+		   msg);
+	  }
 
-	    p = strchr (arg, ' ');
+	p = strchr (arg, ' ');
 
-	    if (p)
-	      len = p - arg;
-	    else
-	      len = strlen (arg);
+	if (p)
+	  len = p - arg;
+	else
+	  len = strlen (arg);
 
-	    nmatches = 0;
-	    for (i = 0; c->enums[i]; i++)
-	      if (strncmp (arg, c->enums[i], len) == 0)
+	nmatches = 0;
+	for (i = 0; c->enums[i]; i++)
+	  if (strncmp (arg, c->enums[i], len) == 0)
+	    {
+	      if (c->enums[i][len] == '\0')
 		{
-		  if (c->enums[i][len] == '\0')
-		    {
-		      match = c->enums[i];
-		      nmatches = 1;
-		      break; /* Exact match.  */
-		    }
-		  else
-		    {
-		      match = c->enums[i];
-		      nmatches++;
-		    }
+		  match = c->enums[i];
+		  nmatches = 1;
+		  break; /* Exact match.  */
 		}
+	      else
+		{
+		  match = c->enums[i];
+		  nmatches++;
+		}
+	    }
 
-	    if (nmatches <= 0)
-	      error (_("Undefined item: \"%s\"."), arg);
+	if (nmatches <= 0)
+	  error (_("Undefined item: \"%s\"."), arg);
 
-	    if (nmatches > 1)
-	      error (_("Ambiguous item \"%s\"."), arg);
+	if (nmatches > 1)
+	  error (_("Ambiguous item \"%s\"."), arg);
 
-	    if (*(const char **) c->var != match)
-	      {
-		*(const char **) c->var = match;
+	if (*(const char **) c->var != match)
+	  {
+	    *(const char **) c->var = match;
 
-		option_changed = 1;
-	      }
+	    option_changed = 1;
 	  }
-	  break;
-	default:
-	  error (_("gdb internal error: bad var_type in do_setshow_command"));
-	}
-      c->func (c, NULL, from_tty);
-      if (deprecated_set_hook)
-	deprecated_set_hook (c);
+      }
+      break;
+    default:
+      error (_("gdb internal error: bad var_type in do_setshow_command"));
+    }
+  c->func (c, NULL, from_tty);
+  if (deprecated_set_hook)
+    deprecated_set_hook (c);
 
   return option_changed;
 }
@@ -384,97 +384,95 @@ void
 do_show_command (char *arg, int from_tty, struct cmd_list_element *c)
 {
   struct ui_out *uiout = current_uiout;
+  struct cleanup *old_chain;
+  struct ui_file *stb;
 
   gdb_assert (c->type == show_cmd);
-  {
-      struct cleanup *old_chain;
-      struct ui_file *stb;
 
-      stb = mem_fileopen ();
-      old_chain = make_cleanup_ui_file_delete (stb);
+  stb = mem_fileopen ();
+  old_chain = make_cleanup_ui_file_delete (stb);
 
-      /* Possibly call the pre hook.  */
-      if (c->pre_show_hook)
-	(c->pre_show_hook) (c);
+  /* Possibly call the pre hook.  */
+  if (c->pre_show_hook)
+    (c->pre_show_hook) (c);
 
-      switch (c->var_type)
+  switch (c->var_type)
+    {
+    case var_string:
+      if (*(char **) c->var)
+	fputstr_filtered (*(char **) c->var, '"', stb);
+      break;
+    case var_string_noescape:
+    case var_optional_filename:
+    case var_filename:
+    case var_enum:
+      if (*(char **) c->var)
+	fputs_filtered (*(char **) c->var, stb);
+      break;
+    case var_boolean:
+      fputs_filtered (*(int *) c->var ? "on" : "off", stb);
+      break;
+    case var_auto_boolean:
+      switch (*(enum auto_boolean*) c->var)
 	{
-	case var_string:
-	  if (*(char **) c->var)
-	    fputstr_filtered (*(char **) c->var, '"', stb);
-	  break;
-	case var_string_noescape:
-	case var_optional_filename:
-	case var_filename:
-	case var_enum:
-	  if (*(char **) c->var)
-	    fputs_filtered (*(char **) c->var, stb);
-	  break;
-	case var_boolean:
-	  fputs_filtered (*(int *) c->var ? "on" : "off", stb);
+	case AUTO_BOOLEAN_TRUE:
+	  fputs_filtered ("on", stb);
 	  break;
-	case var_auto_boolean:
-	  switch (*(enum auto_boolean*) c->var)
-	    {
-	    case AUTO_BOOLEAN_TRUE:
-	      fputs_filtered ("on", stb);
-	      break;
-	    case AUTO_BOOLEAN_FALSE:
-	      fputs_filtered ("off", stb);
-	      break;
-	    case AUTO_BOOLEAN_AUTO:
-	      fputs_filtered ("auto", stb);
-	      break;
-	    default:
-	      internal_error (__FILE__, __LINE__,
-			      _("do_setshow_command: "
-				"invalid var_auto_boolean"));
-	      break;
-	    }
+	case AUTO_BOOLEAN_FALSE:
+	  fputs_filtered ("off", stb);
 	  break;
-	case var_uinteger:
-	case var_zuinteger:
-	  if (c->var_type == var_uinteger
-	      && *(unsigned int *) c->var == UINT_MAX)
-	    fputs_filtered ("unlimited", stb);
-	  else
-	    fprintf_filtered (stb, "%u", *(unsigned int *) c->var);
+	case AUTO_BOOLEAN_AUTO:
+	  fputs_filtered ("auto", stb);
 	  break;
-	case var_integer:
-	case var_zinteger:
-	  if (c->var_type == var_integer
-	      && *(int *) c->var == INT_MAX)
-	    fputs_filtered ("unlimited", stb);
-	  else
-	    fprintf_filtered (stb, "%d", *(int *) c->var);
-	  break;
-
 	default:
-	  error (_("gdb internal error: bad var_type in do_setshow_command"));
+	  internal_error (__FILE__, __LINE__,
+			  _("do_show_command: "
+			    "invalid var_auto_boolean"));
+	  break;
 	}
+      break;
+    case var_uinteger:
+    case var_zuinteger:
+      if (c->var_type == var_uinteger
+	  && *(unsigned int *) c->var == UINT_MAX)
+	fputs_filtered ("unlimited", stb);
+      else
+	fprintf_filtered (stb, "%u", *(unsigned int *) c->var);
+      break;
+    case var_integer:
+    case var_zinteger:
+      if (c->var_type == var_integer
+	  && *(int *) c->var == INT_MAX)
+	fputs_filtered ("unlimited", stb);
+      else
+	fprintf_filtered (stb, "%d", *(int *) c->var);
+      break;
 
+    default:
+      error (_("gdb internal error: bad var_type in do_show_command"));
+    }
 
-      /* FIXME: cagney/2005-02-10: Need to split this in half: code to
-	 convert the value into a string (esentially the above); and
-	 code to print the value out.  For the latter there should be
-	 MI and CLI specific versions.  */
 
-      if (ui_out_is_mi_like_p (uiout))
-	ui_out_field_stream (uiout, "value", stb);
-      else
-	{
-	  char *value = ui_file_xstrdup (stb, NULL);
+  /* FIXME: cagney/2005-02-10: Need to split this in half: code to
+     convert the value into a string (esentially the above); and
+     code to print the value out.  For the latter there should be
+     MI and CLI specific versions.  */
 
-	  make_cleanup (xfree, value);
-	  if (c->show_value_func != NULL)
-	    c->show_value_func (gdb_stdout, from_tty, c, value);
-	  else
-	    deprecated_show_value_hack (gdb_stdout, from_tty, c, value);
-	}
-      do_cleanups (old_chain);
+  if (ui_out_is_mi_like_p (uiout))
+    ui_out_field_stream (uiout, "value", stb);
+  else
+    {
+      char *value = ui_file_xstrdup (stb, NULL);
 
-  c->func (c, NULL, from_tty);
+      make_cleanup (xfree, value);
+      if (c->show_value_func != NULL)
+	c->show_value_func (gdb_stdout, from_tty, c, value);
+      else
+	deprecated_show_value_hack (gdb_stdout, from_tty, c, value);
     }
+  do_cleanups (old_chain);
+
+  c->func (c, NULL, from_tty);
 }
 
 /* Show all the settings in a list of show commands.  */
-- 
1.7.7.6


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

* [PATCH 5/6] notify in string_cmd and trace-notes.
  2012-07-27 15:23 [RCF 0/6 V2] MI notification of command option change Yao Qi
                   ` (3 preceding siblings ...)
  2012-07-27 15:23 ` [PATCH 1/6] new observer command_option_changed Yao Qi
@ 2012-07-27 15:23 ` Yao Qi
  2012-07-27 15:23 ` [PATCH 4/6] notify in boolean_cmd and circular-trace-buffer Yao Qi
  2012-07-27 17:07 ` [RCF 0/6 V2] MI notification of command option change Tom Tromey
  6 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2012-07-27 15:23 UTC (permalink / raw)
  To: gdb-patches

Hi,
this patch is to change function add_setshow_string_cmd to return
'struct cmd_list_element *', and register command "trace-notes".
A new test case is added for it.

gdb:

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

	* cli/cli-decode.c (add_setshow_string_cmd): Return 'struct
	cmd_list_element *'.
	* command.h: Update declaration.
	* tracepoint.c (_initialize_tracepoint): Get the return value of
	add_setshow_string_cmd and call set_cmd_notify.

gdb/testsuite:

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

	* gdb.mi/mi-cmd-opt-changed.exp (test_command_option_change): Test for command
	'set trace-notes'.
---
 gdb/cli/cli-decode.c                        |   11 ++++++++---
 gdb/command.h                               |   21 +++++++++++----------
 gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp |   11 +++++++++++
 gdb/tracepoint.c                            |    9 +++++----
 4 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index c185575..1778030 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -520,8 +520,9 @@ add_setshow_filename_cmd (char *name, enum command_class class,
 }
 
 /* Add element named NAME to both the set and show command LISTs (the
-   list for set/show or some sublist thereof).  */
-void
+   list for set/show or some sublist thereof).  Return the 'set'
+   command handler.  */
+struct cmd_list_element *
 add_setshow_string_cmd (char *name, enum command_class class,
 			char **var,
 			const char *set_doc, const char *show_doc,
@@ -531,11 +532,15 @@ add_setshow_string_cmd (char *name, enum command_class class,
 			struct cmd_list_element **set_list,
 			struct cmd_list_element **show_list)
 {
+  struct cmd_list_element *c;
+
   add_setshow_cmd_full (name, class, var_string, var,
 			set_doc, show_doc, help_doc,
 			set_func, show_func,
 			set_list, show_list,
-			NULL, NULL);
+			&c, NULL);
+
+  return c;
 }
 
 /* Add element named NAME to both the set and show command LISTs (the
diff --git a/gdb/command.h b/gdb/command.h
index 692a16f..4f92d8e 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -281,16 +281,17 @@ extern void add_setshow_filename_cmd (char *name,
 				      struct cmd_list_element **set_list,
 				      struct cmd_list_element **show_list);
 
-extern void add_setshow_string_cmd (char *name,
-				    enum command_class class,
-				    char **var,
-				    const char *set_doc,
-				    const char *show_doc,
-				    const char *help_doc,
-				    cmd_sfunc_ftype *set_func,
-				    show_value_ftype *show_func,
-				    struct cmd_list_element **set_list,
-				    struct cmd_list_element **show_list);
+extern struct cmd_list_element *
+  add_setshow_string_cmd (char *name,
+			  enum command_class class,
+			  char **var,
+			  const char *set_doc,
+			  const char *show_doc,
+			  const char *help_doc,
+			  cmd_sfunc_ftype *set_func,
+			  show_value_ftype *show_func,
+			  struct cmd_list_element **set_list,
+			  struct cmd_list_element **show_list);
 
 extern void add_setshow_string_noescape_cmd (char *name,
 					     enum command_class class,
diff --git a/gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp b/gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp
index e348483..adb3265 100644
--- a/gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp
+++ b/gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp
@@ -68,6 +68,17 @@ proc test_command_option_change { } { with_test_prefix "cmd option" {
 	    "\"set ${command}\" no event"
     }
 
+
+    foreach command { "trace-notes" } {
+	foreach str_opt { "foo" "bar" } {
+	    mi_gdb_test "set ${command} ${str_opt}" \
+		".*=option-changed,option=\"${command}\",value=\"${str_opt}\".*\\^done" \
+		"\"set ${command} ${str_opt}\""
+	}
+	mi_gdb_test "set ${command} bar" \
+	    "\\&\"set ${command} bar\\\\n\"\r\n(\\&\"warning.*)\\^done" \
+	    "\"set ${command} bar\" no event"
+    }
     mi_gdb_exit
 }}
 
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index e7db0f6..61335b7 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -5364,12 +5364,13 @@ Show the user name to use for current and future trace runs"), NULL,
 			  set_trace_user, NULL,
 			  &setlist, &showlist);
 
-  add_setshow_string_cmd ("trace-notes", class_trace,
-			  &trace_notes, _("\
+  c = add_setshow_string_cmd ("trace-notes", class_trace,
+			      &trace_notes, _("\
 Set notes string to use for current and future trace runs"), _("\
 Show the notes string to use for current and future trace runs"), NULL,
-			  set_trace_notes, NULL,
-			  &setlist, &showlist);
+			      set_trace_notes, NULL,
+			      &setlist, &showlist);
+  set_cmd_notify (c);
 
   add_setshow_string_cmd ("trace-stop-notes", class_trace,
 			  &trace_stop_notes, _("\
-- 
1.7.7.6


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

* [PATCH 3/6] notify in enum_cmd and scheduler-locking.
  2012-07-27 15:23 [RCF 0/6 V2] MI notification of command option change Yao Qi
@ 2012-07-27 15:23 ` Yao Qi
  2012-07-27 15:23 ` [PATCH 6/6] code indentation Yao Qi
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2012-07-27 15:23 UTC (permalink / raw)
  To: gdb-patches

Hi,
This patch is to change add_setshow_enum_cmd to return
'struct cmd_list_element *' for set command.  If the command is
set by 'set_cmd_notify', a MI notification is sent to MI frontend
when the option of command is changed.  We choose "scheduler-locking"
as one example of "enum" command, and write a test case for it.

This is V2, compared with V1, I change add_setshow_enum_cmd to
return set command handler, and use 'set_cmd_notify' to set command.

gdb:

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

	* cli/cli-decode.c (set_cmd_notify): New.
	(add_setshow_enum_cmd): Return 'struct cmd_list_element *' for
	set command.
	* command.h: Declare set_cmd_notify.  Update the declaration of
	add_setshow_enum_cmd.
	* infrun.c (_initialize_infrun): Get the return value of
	add_setshow_enum_cmd and call set_cmd_notify.

gdb/testsuite:

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

	* gdb.mi/mi-cmd-opt-changed.exp: New.
---
 gdb/cli/cli-decode.c                        |   16 ++++++-
 gdb/command.h                               |   25 ++++++-----
 gdb/infrun.c                                |   13 ++++--
 gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp |   59 +++++++++++++++++++++++++++
 4 files changed, 95 insertions(+), 18 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 35c7e1e..2dd4b1f 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -323,6 +323,15 @@ empty_sfunc (char *args, int from_tty, struct cmd_list_element *c)
 {
 }
 
+/* The observer 'command_option_changed' will be notified when command
+   CMD option is changed.  */
+
+void
+set_cmd_notify (struct cmd_list_element *cmd)
+{
+  cmd->notify_observer_p = 1;
+}
+
 /* Add element named NAME to command list LIST (the list for set/show
    or some sublist thereof).
    TYPE is set_cmd or show_cmd.
@@ -406,9 +415,10 @@ add_setshow_cmd_full (char *name,
 /* Add element named NAME to command list LIST (the list for set or
    some sublist thereof).  CLASS is as in add_cmd.  ENUMLIST is a list
    of strings which may follow NAME.  VAR is address of the variable
-   which will contain the matching string (from ENUMLIST).  */
+   which will contain the matching string (from ENUMLIST).  Return the
+   'set' command handler.  */
 
-void
+struct cmd_list_element *
 add_setshow_enum_cmd (char *name,
 		      enum command_class class,
 		      const char *const *enumlist,
@@ -429,6 +439,8 @@ add_setshow_enum_cmd (char *name,
 			set_list, show_list,
 			&c, NULL);
   c->enums = enumlist;
+
+  return c;
 }
 
 const char * const auto_boolean_enums[] = { "on", "off", "auto", NULL };
diff --git a/gdb/command.h b/gdb/command.h
index 88895bb..3f3703c 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -193,6 +193,8 @@ extern struct cmd_list_element *deprecate_cmd (struct cmd_list_element *,
 
 extern void deprecated_cmd_warning (char **);
 
+extern void set_cmd_notify (struct cmd_list_element *);
+
 extern int lookup_cmd_composition (char *text,
 				   struct cmd_list_element **alias,
 				   struct cmd_list_element **prefix_cmd,
@@ -233,17 +235,18 @@ typedef void (show_value_ftype) (struct ui_file *file,
    instead print the value out directly.  */
 extern show_value_ftype deprecated_show_value_hack;
 
-extern void add_setshow_enum_cmd (char *name,
-				  enum command_class class,
-				  const char *const *enumlist,
-				  const char **var,
-				  const char *set_doc,
-				  const char *show_doc,
-				  const char *help_doc,
-				  cmd_sfunc_ftype *set_func,
-				  show_value_ftype *show_func,
-				  struct cmd_list_element **set_list,
-				  struct cmd_list_element **show_list);
+extern struct cmd_list_element *
+  add_setshow_enum_cmd (char *name,
+			enum command_class class,
+			const char *const *enumlist,
+			const char **var,
+			const char *set_doc,
+			const char *show_doc,
+			const char *help_doc,
+			cmd_sfunc_ftype *set_func,
+			show_value_ftype *show_func,
+			struct cmd_list_element **set_list,
+			struct cmd_list_element **show_list);
 
 extern void add_setshow_auto_boolean_cmd (char *name,
 					  enum command_class class,
diff --git a/gdb/infrun.c b/gdb/infrun.c
index efc4162..39ec552 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7059,6 +7059,7 @@ _initialize_infrun (void)
 {
   int i;
   int numsigs;
+  struct cmd_list_element *c;
 
   add_info ("signals", signals_info, _("\
 What debugger does when program gets various signals.\n\
@@ -7250,8 +7251,8 @@ By default, the debugger will use the same inferior."),
 			show_follow_exec_mode_string,
 			&setlist, &showlist);
 
-  add_setshow_enum_cmd ("scheduler-locking", class_run, 
-			scheduler_enums, &scheduler_mode, _("\
+  c = add_setshow_enum_cmd ("scheduler-locking", class_run, 
+			    scheduler_enums, &scheduler_mode, _("\
 Set mode for locking scheduler during execution."), _("\
 Show mode for locking scheduler during execution."), _("\
 off  == no locking (threads may preempt at any time)\n\
@@ -7259,9 +7260,11 @@ on   == full locking (no thread except the current thread may run)\n\
 step == scheduler locked during every single-step operation.\n\
 	In this mode, no other thread may run during a step command.\n\
 	Other threads may run while stepping over a function call ('next')."), 
-			set_schedlock_func,	/* traps on target vector */
-			show_scheduler_mode,
-			&setlist, &showlist);
+			    /* traps on target vector */
+			    set_schedlock_func,
+			    show_scheduler_mode,
+			    &setlist, &showlist);
+  set_cmd_notify (c);
 
   add_setshow_boolean_cmd ("schedule-multiple", class_run, &sched_multi, _("\
 Set mode for resuming threads of all processes."), _("\
diff --git a/gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp b/gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp
new file mode 100644
index 0000000..c8011a9
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp
@@ -0,0 +1,59 @@
+# Copyright 2012 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile basics.c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested mi-cmd-opt-changed.exp
+     return -1
+}
+
+proc test_command_option_change { } { with_test_prefix "cmd option" {
+    if [mi_gdb_start] {
+	return
+    }
+    mi_run_to_main
+
+    foreach opt { "on" "off" "step" } {
+	mi_gdb_test "set scheduler-locking ${opt}" \
+	    ".*=option-changed,option=\"scheduler-locking\",value=\"${opt}\".*\\^done" \
+	    "\"set scheduler-locking ${opt}\""
+    }
+    foreach opt { "on" "off" "step" } {
+	mi_gdb_test "interpreter-exec console \"set scheduler-locking ${opt}\"" \
+	    ".*=option-changed,option=\"scheduler-locking\",value=\"${opt}\".*\\^done" \
+	    "interpreter-exec \"set scheduler-locking ${opt}\""
+    }
+    # Don't emit MI notification for request from MI.
+    mi_gdb_test "-gdb-set scheduler-locking on" \
+	{\^done} \
+	"\"set scheduler-locking on\" no event (requested by MI)"
+
+    mi_gdb_test "interpreter-exec mi \"-gdb-set scheduler-locking step\"" \
+	"\\&\"interpreter-exec mi .*\"-gdb-set scheduler-locking step.*\"\\\\n\"\r\n\\^done\r\n\\^done" \
+	"\"set scheduler-locking step\" no event (requested by MI interp)"
+    mi_gdb_test "set scheduler-locking step" \
+	"\\&\"set scheduler-locking step\\\\n\"\r\n\\^done" \
+	"\"set scheduler-locking stepr\" no event"
+
+    mi_gdb_exit
+}}
+
+test_command_option_change
+
+return 0
-- 
1.7.7.6


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

* [RCF 0/6 V2] MI notification of command option change
@ 2012-07-27 15:23 Yao Qi
  2012-07-27 15:23 ` [PATCH 3/6] notify in enum_cmd and scheduler-locking Yao Qi
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Yao Qi @ 2012-07-27 15:23 UTC (permalink / raw)
  To: gdb-patches

Hi,
This is the V2 of this patch set.  Compared with V1, some decisions
are made:

  1.  Keep the command-registration functions unchanged (discard
my xxx_wit_notif functions).
  2.  Don't emit option change of all gdb commands.  We have to
set 'reporting' commands via function 'set_cmd_notify', otherwise
commands are still 'no-reporting'.  I believe that frontends will
request more and more options changes, but changes of some gdb
options are useless to frontends.  It is noisy to send all option
changes.

Tom and Pedro's review comments are addressed in this version.


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

* [PATCH 4/6] notify in boolean_cmd and circular-trace-buffer.
  2012-07-27 15:23 [RCF 0/6 V2] MI notification of command option change Yao Qi
                   ` (4 preceding siblings ...)
  2012-07-27 15:23 ` [PATCH 5/6] notify in string_cmd and trace-notes Yao Qi
@ 2012-07-27 15:23 ` Yao Qi
  2012-07-27 17:07 ` [RCF 0/6 V2] MI notification of command option change Tom Tromey
  6 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2012-07-27 15:23 UTC (permalink / raw)
  To: gdb-patches

Hi,
Similar to previous patch, this patch is to add a new function
add_setshow_boolean_cmd_with_notif, and register command
"circular-trace-buffer" as an example to boolean command.  A new
test case is added for it.

gdb:

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

	* cli/cli-decode.c (add_setshow_auto_boolean_cmd): Move
	'boolean_enums' out.
	Return 'struct cmd_list_element *' for set command.
	* command.h: Update declaration of add_setshow_auto_boolean_cmd.
	* tracepoint.c (_initialize_tracepoint): Get the return value
	of add_setshow_auto_boolean_cmd can call set_cmd_notify.

gdb/testsuite:

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

	* gdb.mi/mi-cmd-opt-changed.exp (test_command_option_change): Test for command
	'circular-trace-buffer'.
---
 gdb/cli/cli-decode.c                        |    9 ++++++---
 gdb/command.h                               |   19 ++++++++++---------
 gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp |   17 +++++++++++++++++
 gdb/tracepoint.c                            |   13 +++++++------
 4 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 2dd4b1f..c185575 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -470,11 +470,13 @@ add_setshow_auto_boolean_cmd (char *name,
   c->enums = auto_boolean_enums;
 }
 
+static const char *boolean_enums[] = { "on", "off", NULL };
 /* Add element named NAME to both the set and show command LISTs (the
    list for set/show or some sublist thereof).  CLASS is as in
    add_cmd.  VAR is address of the variable which will contain the
-   value.  SET_DOC and SHOW_DOC are the documentation strings.  */
-void
+   value.  SET_DOC and SHOW_DOC are the documentation strings.  Return
+   the 'set' command handler.*/
+struct cmd_list_element *
 add_setshow_boolean_cmd (char *name, enum command_class class, int *var,
 			 const char *set_doc, const char *show_doc,
 			 const char *help_doc,
@@ -483,7 +485,6 @@ add_setshow_boolean_cmd (char *name, enum command_class class, int *var,
 			 struct cmd_list_element **set_list,
 			 struct cmd_list_element **show_list)
 {
-  static const char *boolean_enums[] = { "on", "off", NULL };
   struct cmd_list_element *c;
 
   add_setshow_cmd_full (name, class, var_boolean, var,
@@ -492,6 +493,8 @@ add_setshow_boolean_cmd (char *name, enum command_class class, int *var,
 			set_list, show_list,
 			&c, NULL);
   c->enums = boolean_enums;
+
+  return c;
 }
 
 /* Add element named NAME to both the set and show command LISTs (the
diff --git a/gdb/command.h b/gdb/command.h
index 3f3703c..692a16f 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -259,15 +259,16 @@ extern void add_setshow_auto_boolean_cmd (char *name,
 					  struct cmd_list_element **set_list,
 					  struct cmd_list_element **show_list);
 
-extern void add_setshow_boolean_cmd (char *name,
-				     enum command_class class,
-				     int *var,
-				     const char *set_doc, const char *show_doc,
-				     const char *help_doc,
-				     cmd_sfunc_ftype *set_func,
-				     show_value_ftype *show_func,
-				     struct cmd_list_element **set_list,
-				     struct cmd_list_element **show_list);
+extern struct cmd_list_element *
+  add_setshow_boolean_cmd (char *name,
+			   enum command_class class,
+			   int *var,
+			   const char *set_doc, const char *show_doc,
+			   const char *help_doc,
+			   cmd_sfunc_ftype *set_func,
+			   show_value_ftype *show_func,
+			   struct cmd_list_element **set_list,
+			   struct cmd_list_element **show_list);
 
 extern void add_setshow_filename_cmd (char *name,
 				      enum command_class class,
diff --git a/gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp b/gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp
index c8011a9..e348483 100644
--- a/gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp
+++ b/gdb/testsuite/gdb.mi/mi-cmd-opt-changed.exp
@@ -51,6 +51,23 @@ proc test_command_option_change { } { with_test_prefix "cmd option" {
 	"\\&\"set scheduler-locking step\\\\n\"\r\n\\^done" \
 	"\"set scheduler-locking stepr\" no event"
 
+
+    foreach command { "circular-trace-buffer" } {
+
+	# The default value of each command option may be different, so we first
+	# set it to 'off', and this may or may not trigger MI notification.
+	mi_gdb_test "set ${command} off" ".*\\^done" "\"set ${command}\" warmup"
+
+	foreach boolean_opt { "on" "off" } {
+	    mi_gdb_test "set ${command} ${boolean_opt}" \
+		".*=option-changed,option=\"${command}\",value=\"${boolean_opt}\".*\\^done" \
+		"\"set ${command} ${boolean_opt}\""
+	}
+	mi_gdb_test "set ${command} off" \
+	    "\\&\"set ${command} off\\\\n\"\r\n\\^done" \
+	    "\"set ${command}\" no event"
+    }
+
     mi_gdb_exit
 }}
 
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 8c8d4a8..e7db0f6 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -5344,17 +5344,18 @@ trace data collected in the meantime."),
 			   &setlist,
 			   &showlist);
 
-  add_setshow_boolean_cmd ("circular-trace-buffer", no_class,
-			   &circular_trace_buffer, _("\
+  c = add_setshow_boolean_cmd ("circular-trace-buffer", no_class,
+			       &circular_trace_buffer, _("\
 Set target's use of circular trace buffer."), _("\
 Show target's use of circular trace buffer."), _("\
 Use this to make the trace buffer into a circular buffer,\n\
 which will discard traceframes (oldest first) instead of filling\n\
 up and stopping the trace run."),
-			   set_circular_trace_buffer,
-			   NULL,
-			   &setlist,
-			   &showlist);
+			       set_circular_trace_buffer,
+			       NULL,
+			       &setlist,
+			       &showlist);
+  set_cmd_notify (c);
 
   add_setshow_string_cmd ("trace-user", class_trace,
 			  &trace_user, _("\
-- 
1.7.7.6


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

* [PATCH 1/6] new observer command_option_changed.
  2012-07-27 15:23 [RCF 0/6 V2] MI notification of command option change Yao Qi
                   ` (2 preceding siblings ...)
  2012-07-27 15:23 ` [PATCH 2/6] attach to command_option-changed observer Yao Qi
@ 2012-07-27 15:23 ` Yao Qi
  2012-07-27 17:56   ` Tom Tromey
  2012-07-27 15:23 ` [PATCH 5/6] notify in string_cmd and trace-notes Yao Qi
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Yao Qi @ 2012-07-27 15:23 UTC (permalink / raw)
  To: gdb-patches

Hi,
This patch is to add an extra field 'notify_observer_p' to identify
the command will notify observer 'command_option_changed', and teach
GDB to check the change of command option, and notify observer.

Compared with V1, V2 has following changes,
  1.  Move notifying 'command_option_changed' observer out of
do_setshow_command to its caller, execute_command, so that we can
get the full command line to send MI notification for multi-word
options.
  2.  Split do_setshow_command to do_set_command and do_show_command.

Note that in order to make patch easier to read, the indnetation is
not adjusted.  Indentation is fixed in a separate patch.

gdb:

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

	* cli/cli-decode.c (add_cmd): Set field 'notify_observer_p' to 0.
	(add_setshow_auto_boolean_cmd): Move auto_boolean_enums out.
	Remove 'static'.
	* cli/cli-decode.h (struct cmd_list_element) <notify_observer_p>:
	New field.
	Declare 'auto_boolean_enums'.
	* cli/cli-setshow.c (do_setshow_command): Split it to ...
	(do_set_command, do_show_command): ... them.  New.
	(cmd_show_list): Caller update.
	* auto-load.c (set_auto_load_cmd): Likewise.
	* remote.c (show_remote_cmd): Likewise.
	* cli/cli-setshow.h: Update declarations.
	* top.c (execute_command): Call do_set_command and
	call observer_notify_command_option_changed if it return true.

gdb/doc:

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

	* observer.texi: New observer command_option_changed.
---
 gdb/auto-load.c       |    2 +-
 gdb/cli/cli-decode.c  |    4 +-
 gdb/cli/cli-decode.h  |    4 +
 gdb/cli/cli-setshow.c |  164 ++++++++++++++++++++++++++++++++++++------------
 gdb/cli/cli-setshow.h |   10 +--
 gdb/doc/observer.texi |    8 +++
 gdb/remote.c          |    2 +-
 gdb/top.c             |   22 ++++++-
 8 files changed, 163 insertions(+), 53 deletions(-)

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 87dd1e4..471a8a7 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -1024,7 +1024,7 @@ set_auto_load_cmd (char *args, int from_tty)
     if (list->var_type == var_boolean)
       {
 	gdb_assert (list->type == set_cmd);
-	do_setshow_command (args, from_tty, list);
+	do_set_command (args, from_tty, list);
       }
 }
 
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index c337b43..35c7e1e 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -203,6 +203,7 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
   c->user_commands = NULL;
   c->cmd_pointer = NULL;
   c->alias_chain = NULL;
+  c->notify_observer_p = 0;
 
   return c;
 }
@@ -430,6 +431,8 @@ add_setshow_enum_cmd (char *name,
   c->enums = enumlist;
 }
 
+const char * const auto_boolean_enums[] = { "on", "off", "auto", NULL };
+
 /* Add an auto-boolean command named NAME to both the set and show
    command list lists.  CLASS is as in add_cmd.  VAR is address of the
    variable which will contain the value.  DOC is the documentation
@@ -445,7 +448,6 @@ add_setshow_auto_boolean_cmd (char *name,
 			      struct cmd_list_element **set_list,
 			      struct cmd_list_element **show_list)
 {
-  static const char *auto_boolean_enums[] = { "on", "off", "auto", NULL };
   struct cmd_list_element *c;
 
   add_setshow_cmd_full (name, class, var_auto_boolean, var,
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index b5e0790..ae5d0a1 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -210,6 +210,9 @@ struct cmd_list_element
 
     /* Link pointer for aliases on an alias list.  */
     struct cmd_list_element *alias_chain;
+
+    /* Notify 'command_option_changed' observer if it is true.  */
+    int notify_observer_p;
   };
 
 extern void help_cmd_list (struct cmd_list_element *, enum command_class,
@@ -232,5 +235,6 @@ extern void not_just_help_class_command (char *arg, int from_tty);
 
 extern void print_doc_line (struct ui_file *, char *);
 
+extern const char * const auto_boolean_enums[];
 
 #endif /* !defined (CLI_DECODE_H) */
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 7ffb89e..17c336e 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -116,18 +116,20 @@ deprecated_show_value_hack (struct ui_file *ignore_file,
     }
 }
 
-/* Do a "set" or "show" command.  ARG is NULL if no argument, or the
+/* Do a "set" command.  ARG is NULL if no argument, or the
    text of the argument, and FROM_TTY is nonzero if this command is
    being entered directly by the user (i.e. these are just like any
-   other command).  C is the command list element for the command.  */
+   other command).  C is the command list element for the command.  Return
+   true if command option is changed, otherwise return false.  */
 
-void
-do_setshow_command (char *arg, int from_tty, struct cmd_list_element *c)
+int
+do_set_command (char *arg, int from_tty, struct cmd_list_element *c)
 {
-  struct ui_out *uiout = current_uiout;
+  /* A flag to indicate the option is changed or not.  */
+  int option_changed = 0;
+
+  gdb_assert (c->type == set_cmd);
 
-  if (c->type == set_cmd)
-    {
       switch (c->var_type)
 	{
 	case var_string:
@@ -170,50 +172,106 @@ do_setshow_command (char *arg, int from_tty, struct cmd_list_element *c)
 #endif
 	    *q++ = '\0';
 	    new = (char *) xrealloc (new, q - new);
-	    xfree (*(char **) c->var);
-	    *(char **) c->var = new;
+
+	    if (*(char **) c->var == NULL
+		|| strcmp (*(char **) c->var, new) != 0)
+	      {
+		xfree (*(char **) c->var);
+		*(char **) c->var = new;
+
+		option_changed = 1;
+	      }
+	    else
+	      xfree (new);
 	  }
 	  break;
 	case var_string_noescape:
 	  if (arg == NULL)
 	    arg = "";
-	  xfree (*(char **) c->var);
-	  *(char **) c->var = xstrdup (arg);
+
+	  if (*(char **) c->var == NULL || strcmp (*(char **) c->var, arg) != 0)
+	    {
+	      xfree (*(char **) c->var);
+	      *(char **) c->var = xstrdup (arg);
+
+	      option_changed = 1;
+	    }
 	  break;
 	case var_filename:
 	  if (arg == NULL)
 	    error_no_arg (_("filename to set it to."));
 	  /* FALLTHROUGH */
 	case var_optional_filename:
-	  xfree (*(char **) c->var);
+	  {
+	    char *val = NULL;
 
-	  if (arg != NULL)
-	    {
-	      /* Clear trailing whitespace of filename.  */
-	      char *ptr = arg + strlen (arg) - 1;
+	    if (arg != NULL)
+	      {
+		/* Clear trailing whitespace of filename.  */
+		char *ptr = arg + strlen (arg) - 1;
 
-	      while (ptr >= arg && (*ptr == ' ' || *ptr == '\t'))
-		ptr--;
-	      *(ptr + 1) = '\0';
+		while (ptr >= arg && (*ptr == ' ' || *ptr == '\t'))
+		  ptr--;
+		*(ptr + 1) = '\0';
 
-	      *(char **) c->var = tilde_expand (arg);
-	    }
-	  else
-	    *(char **) c->var = xstrdup ("");
+		val = tilde_expand (arg);
+	      }
+	    else
+	      val = xstrdup ("");
+
+	    if (*(char **) c->var == NULL
+		|| strcmp (*(char **) c->var, val) != 0)
+	      {
+		xfree (*(char **) c->var);
+		*(char **) c->var = val;
+
+		option_changed = 1;
+	      }
+	    else
+	      xfree (val);
+	  }
 	  break;
 	case var_boolean:
-	  *(int *) c->var = parse_binary_operation (arg);
+	  {
+	    int val = parse_binary_operation (arg);
+
+	    if (val != *(int *) c->var)
+	      {
+		*(int *) c->var = val;
+
+		option_changed = 1;
+	      }
+	  }
 	  break;
 	case var_auto_boolean:
-	  *(enum auto_boolean *) c->var = parse_auto_binary_operation (arg);
+	  {
+	    enum auto_boolean val = parse_auto_binary_operation (arg);
+
+	    if (*(enum auto_boolean *) c->var != val)
+	      {
+		*(enum auto_boolean *) c->var = val;
+
+		option_changed = 1;
+	      }
+	  }
 	  break;
 	case var_uinteger:
 	case var_zuinteger:
 	  if (arg == NULL)
 	    error_no_arg (_("integer to set it to."));
-	  *(unsigned int *) c->var = parse_and_eval_long (arg);
-	  if (c->var_type == var_uinteger && *(unsigned int *) c->var == 0)
-	    *(unsigned int *) c->var = UINT_MAX;
+	  {
+	    unsigned int val = parse_and_eval_long (arg);
+
+	    if (c->var_type == var_uinteger && val == 0)
+	      val = UINT_MAX;
+
+	    if (*(unsigned int *) c->var != val)
+	      {
+		*(unsigned int *) c->var = val;
+
+		option_changed = 1;
+	      }
+	  }
 	  break;
 	case var_integer:
 	case var_zinteger:
@@ -224,11 +282,17 @@ do_setshow_command (char *arg, int from_tty, struct cmd_list_element *c)
 	      error_no_arg (_("integer to set it to."));
 	    val = parse_and_eval_long (arg);
 	    if (val == 0 && c->var_type == var_integer)
-	      *(int *) c->var = INT_MAX;
+	      val = INT_MAX;
 	    else if (val >= INT_MAX)
 	      error (_("integer %u out of range"), val);
-	    else
-	      *(int *) c->var = val;
+
+
+	    if (*(int *) c->var != val)
+	      {
+		*(int *) c->var = val;
+
+		option_changed = 1;
+	      }
 	    break;
 	  }
 	case var_enum:
@@ -293,15 +357,36 @@ do_setshow_command (char *arg, int from_tty, struct cmd_list_element *c)
 	    if (nmatches > 1)
 	      error (_("Ambiguous item \"%s\"."), arg);
 
-	    *(const char **) c->var = match;
+	    if (*(const char **) c->var != match)
+	      {
+		*(const char **) c->var = match;
+
+		option_changed = 1;
+	      }
 	  }
 	  break;
 	default:
 	  error (_("gdb internal error: bad var_type in do_setshow_command"));
 	}
-    }
-  else if (c->type == show_cmd)
-    {
+      c->func (c, NULL, from_tty);
+      if (deprecated_set_hook)
+	deprecated_set_hook (c);
+
+  return option_changed;
+}
+
+/* Do a "show" command.  ARG is NULL if no argument, or the
+   text of the argument, and FROM_TTY is nonzero if this command is
+   being entered directly by the user (i.e. these are just like any
+   other command).  C is the command list element for the command.  */
+
+void
+do_show_command (char *arg, int from_tty, struct cmd_list_element *c)
+{
+  struct ui_out *uiout = current_uiout;
+
+  gdb_assert (c->type == show_cmd);
+  {
       struct cleanup *old_chain;
       struct ui_file *stb;
 
@@ -387,12 +472,9 @@ do_setshow_command (char *arg, int from_tty, struct cmd_list_element *c)
 	    deprecated_show_value_hack (gdb_stdout, from_tty, c, value);
 	}
       do_cleanups (old_chain);
-    }
-  else
-    error (_("gdb internal error: bad cmd_type in do_setshow_command"));
+
   c->func (c, NULL, from_tty);
-  if (c->type == set_cmd && deprecated_set_hook)
-    deprecated_set_hook (c);
+    }
 }
 
 /* Show all the settings in a list of show commands.  */
@@ -431,7 +513,7 @@ cmd_show_list (struct cmd_list_element *list, int from_tty, char *prefix)
 	      ui_out_field_string (uiout, "name", list->name);
 	      ui_out_text (uiout, ":  ");
 	      if (list->type == show_cmd)
-		do_setshow_command ((char *) NULL, from_tty, list);
+		do_show_command ((char *) NULL, from_tty, list);
 	      else
 		cmd_func (list, NULL, from_tty);
 	      /* Close the tuple.  */
diff --git a/gdb/cli/cli-setshow.h b/gdb/cli/cli-setshow.h
index cb8d2c5..8f43e53 100644
--- a/gdb/cli/cli-setshow.h
+++ b/gdb/cli/cli-setshow.h
@@ -21,12 +21,10 @@ struct cmd_list_element;
 
 /* Exported to cli/cli-cmds.c and gdb/top.c */
 
-/* Do a "set" or "show" command.  ARG is NULL if no argument, or the
-   text of the argument, and FROM_TTY is nonzero if this command is
-   being entered directly by the user (i.e. these are just like any
-   other command).  C is the command list element for the command.  */
-extern void do_setshow_command (char *arg, int from_tty,
-				struct cmd_list_element *c);
+extern int do_set_command (char *arg, int from_tty,
+			   struct cmd_list_element *c);
+extern void do_show_command (char *arg, int from_tty,
+			     struct cmd_list_element *c);
 
 /* Exported to cli/cli-cmds.c and gdb/top.c, language.c and valprint.c */
 
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index 14d4ac3..4c44a80 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -234,6 +234,14 @@ the current top-level prompt.
 Variable gdb_datadir has been set.  The value may not necessarily change.
 @end deftypefun
 
+@deftypefun void command_option_changed (const char *@var{option}, const char *@var{value})
+The option of some @code{set} commands in console are changed.  This
+method is called after a command @code{set @var{option} @var{value}}.
+@var{option} is the option of @code{set} command, and @var{value}
+is the value of changed option.
+
+@end deftypefun
+
 @deftypefun void test_notification (int @var{somearg})
 This observer is used for internal testing.  Do not use.  
 See testsuite/gdb.gdb/observer.exp.
diff --git a/gdb/remote.c b/gdb/remote.c
index fa514dc..87b8921 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11270,7 +11270,7 @@ show_remote_cmd (char *args, int from_tty)
 	ui_out_field_string (uiout, "name", list->name);
 	ui_out_text (uiout, ":  ");
 	if (list->type == show_cmd)
-	  do_setshow_command ((char *) NULL, from_tty, list);
+	  do_show_command ((char *) NULL, from_tty, list);
 	else
 	  cmd_func (list, NULL, from_tty);
 	/* Close the tuple.  */
diff --git a/gdb/top.c b/gdb/top.c
index 061ad48..46fa9c3 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -412,7 +412,7 @@ execute_command (char *p, int from_tty)
 {
   struct cleanup *cleanup_if_error, *cleanup;
   struct cmd_list_element *c;
-  char *line;
+  char *line, *orig;
 
   cleanup_if_error = make_bpstat_clear_actions_cleanup ();
   cleanup = prepare_execute_command ();
@@ -468,14 +468,30 @@ execute_command (char *p, int from_tty)
       /* If this command has been pre-hooked, run the hook first.  */
       execute_cmd_pre_hook (c);
 
+      orig = line;
       if (c->flags & DEPRECATED_WARN_USER)
 	deprecated_cmd_warning (&line);
 
       /* c->user_commands would be NULL in the case of a python command.  */
       if (c->class == class_user && c->user_commands)
 	execute_user_command (c, arg);
-      else if (c->type == set_cmd || c->type == show_cmd)
-	do_setshow_command (arg, from_tty, c);
+      else if (c->type == set_cmd)
+	{
+	  if (do_set_command (arg, from_tty, c) && c->notify_observer_p)
+	    {
+	      /* Skip 'set ' */
+	      int len = arg - orig - 4;
+	      char *option = xmalloc (len);
+
+	      memcpy (option, orig + 4, len - 1);
+	      option[len - 1] = 0;
+
+	      observer_notify_command_option_changed (option, arg);
+	      xfree (option);
+	    }
+	}
+      else if (c->type == show_cmd)
+	do_show_command (arg, from_tty, c);
       else if (!cmd_func_p (c))
 	error (_("That is not a command, just a help topic."));
       else if (deprecated_call_command_hook)
-- 
1.7.7.6


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

* Re: [RCF 0/6 V2] MI notification of command option change
  2012-07-27 15:23 [RCF 0/6 V2] MI notification of command option change Yao Qi
                   ` (5 preceding siblings ...)
  2012-07-27 15:23 ` [PATCH 4/6] notify in boolean_cmd and circular-trace-buffer Yao Qi
@ 2012-07-27 17:07 ` Tom Tromey
  6 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2012-07-27 17:07 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

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

Yao>   2.  Don't emit option change of all gdb commands.  We have to
Yao> set 'reporting' commands via function 'set_cmd_notify', otherwise
Yao> commands are still 'no-reporting'.  I believe that frontends will
Yao> request more and more options changes, but changes of some gdb
Yao> options are useless to frontends.  It is noisy to send all option
Yao> changes.

Yesterday's discussions convinced me that the best route is to report
all changes.

If it is really necessary to throttle changes, then I think there should
be a new MI command so that the MI client can specify which ones it does
or does not want to see.

Tom


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

* Re: [PATCH 2/6] attach to command_option-changed observer.
  2012-07-27 15:23 ` [PATCH 2/6] attach to command_option-changed observer Yao Qi
@ 2012-07-27 17:20   ` Tom Tromey
  2012-07-27 17:44   ` Tom Tromey
  1 sibling, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2012-07-27 17:20 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

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

Yao> +@item =option-changed,option=@var{option},value=@var{value}
Yao> +Reports that an option of the command @code{set @var{option}} is
Yao> +changed to @var{value}.

I think it would be useful to MI client authors if this were a bit more
specific; in particular describing what happens for a multi-word "set"
command like "set print elements 5".

Yao> +  ui_out_redirect (mi_uiout, mi->event_channel);
Yao> +  ui_out_field_string (mi_uiout, "option", option);
Yao> +  ui_out_field_string (mi_uiout, "value", value);
Yao> +  ui_out_redirect (mi_uiout, NULL);

Nice and simple :-)

Yao> +  /* Breakpoint notification suppressed? */
Yao> +  int breakpoint;
Yao> +  /* Command option changed notification suppressed? */
Yao> +  int option_changed;

Two spaces after the "?"s.

Tom


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

* Re: [PATCH 2/6] attach to command_option-changed observer.
  2012-07-27 15:23 ` [PATCH 2/6] attach to command_option-changed observer Yao Qi
  2012-07-27 17:20   ` Tom Tromey
@ 2012-07-27 17:44   ` Tom Tromey
  1 sibling, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2012-07-27 17:44 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

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

Yao> This patch is to attach function 'mi_command_option_changed' to
Yao> observer 'command_option_changed', so that a MI notification
Yao> "=option-changed" is sent to MI frontend.  If the command option
Yao> change is requested from MI, the notification is suppressed.

It occurs to me now that, when we had the naming discussion for the
Python feature related to set/show, we ended up with the name
"parameter" rather than "option" or "variable".  Perhaps it would be
best if MI adopted this same naming convention.

Tom


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

* Re: [PATCH 1/6] new observer command_option_changed.
  2012-07-27 15:23 ` [PATCH 1/6] new observer command_option_changed Yao Qi
@ 2012-07-27 17:56   ` Tom Tromey
  0 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2012-07-27 17:56 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

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

Yao>   1.  Move notifying 'command_option_changed' observer out of
Yao> do_setshow_command to its caller, execute_command, so that we can
Yao> get the full command line to send MI notification for multi-word
Yao> options.

Yao> +	  if (do_set_command (arg, from_tty, c) && c->notify_observer_p)
Yao> +	    {
Yao> +	      /* Skip 'set ' */
Yao> +	      int len = arg - orig - 4;
Yao> +	      char *option = xmalloc (len);
Yao> +
Yao> +	      memcpy (option, orig + 4, len - 1);
Yao> +	      option[len - 1] = 0;
Yao> +
Yao> +	      observer_notify_command_option_changed (option, arg);

What happens here if the user enters an abbreviation for the command?
For example, "set print ele 5" works fine -- but the observer should
still see "print elements" as the argument name.  I think the code above
will do the wrong thing.

do_set_command doesn't always just use the plain value of ARG to set the
parameter.  So, passing that to the observer can sometimes result in the
client being out of sync.  For example, tilde expansion won't be seen.

Likewise, I wonder about "maint" commands like "maint set profile".
Perhaps these could be automatically excluded; maybe based on
class_maintenance?

Tom


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

* Re: [RCF 0/6 V2] MI notification of command option change
  2012-07-27 14:06 Yao Qi
@ 2012-07-27 14:15 ` Yao Qi
  0 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2012-07-27 14:15 UTC (permalink / raw)
  To: gdb-patches

On Friday, July 27, 2012 10:04:54 PM Yao Qi wrote:
>   1.  Kee the command-registration functions unchanged (discard
> my xxx_wit_notif functions).
>   2.  Don't emit option change of *all* gdb commands.  We have to
> set 'reporting' commands via function 'set_cmd_notify', otherwise
> commands are still 'no-reporting'.  I believe that frontends will
> request more and more options changes, but changes of some gdb
> options are useless to frontends.  It is noisy to send all option
> changes.
> 
> Tom and Pedro's review comments are addressed in this version.

I just missed Pedro's comment on 'using array reduces the debugability of 
gdb'.  I'll fix that.  Please ignore this thread.  Sorry for the noise here.

-- 
Yao (齐尧)


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

* [RCF 0/6 V2] MI notification of command option change
@ 2012-07-27 14:06 Yao Qi
  2012-07-27 14:15 ` Yao Qi
  0 siblings, 1 reply; 13+ messages in thread
From: Yao Qi @ 2012-07-27 14:06 UTC (permalink / raw)
  To: gdb-patches


  1.  Kee the command-registration functions unchanged (discard
my xxx_wit_notif functions).
  2.  Don't emit option change of *all* gdb commands.  We have to
set 'reporting' commands via function 'set_cmd_notify', otherwise
commands are still 'no-reporting'.  I believe that frontends will
request more and more options changes, but changes of some gdb
options are useless to frontends.  It is noisy to send all option
changes.

Tom and Pedro's review comments are addressed in this version.


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

end of thread, other threads:[~2012-07-27 17:56 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-27 15:23 [RCF 0/6 V2] MI notification of command option change Yao Qi
2012-07-27 15:23 ` [PATCH 3/6] notify in enum_cmd and scheduler-locking Yao Qi
2012-07-27 15:23 ` [PATCH 6/6] code indentation Yao Qi
2012-07-27 15:23 ` [PATCH 2/6] attach to command_option-changed observer Yao Qi
2012-07-27 17:20   ` Tom Tromey
2012-07-27 17:44   ` Tom Tromey
2012-07-27 15:23 ` [PATCH 1/6] new observer command_option_changed Yao Qi
2012-07-27 17:56   ` Tom Tromey
2012-07-27 15:23 ` [PATCH 5/6] notify in string_cmd and trace-notes Yao Qi
2012-07-27 15:23 ` [PATCH 4/6] notify in boolean_cmd and circular-trace-buffer Yao Qi
2012-07-27 17:07 ` [RCF 0/6 V2] MI notification of command option change Tom Tromey
  -- strict thread matches above, loose matches on Subject: below --
2012-07-27 14:06 Yao Qi
2012-07-27 14:15 ` Yao Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox