[RCF 0/6 V2] MI notification of command option change

[RCF 0/6 V2] MI notification of command option change

[Yao Qi]

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.

[PATCH 6/6] code indentation.

[Yao Qi]

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

[PATCH 3/6] notify in enum_cmd and scheduler-locking.

[Yao Qi]

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

[PATCH 1/6] new observer command_option_changed.

[Yao Qi]

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

Re: [PATCH 1/6] new observer command_option_changed.

[Tom Tromey]

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

[PATCH 5/6] notify in string_cmd and trace-notes.

[Yao Qi]

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

[PATCH 4/6] notify in boolean_cmd and circular-trace-buffer.

[Yao Qi]

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

[PATCH 2/6] attach to command_option-changed observer.

[Yao Qi]

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

Re: [PATCH 2/6] attach to command_option-changed observer.

[Tom Tromey]

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

Re: [PATCH 2/6] attach to command_option-changed observer.

[Tom Tromey]

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

Re: [RCF 0/6 V2] MI notification of command option change

[Tom Tromey]

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

[RFC 0/6] MI notification of command option change

[Yao Qi]

When Eclipse connects to GDB through MI, user is still abel to
type command in console (or telnet session), so the state of GDB
may be changed while Eclipse is not aware of.  GDB is lack of
notifications to MI front end and/or telnet session when the
internal state of GDB is changed (eg. changed by some 'set foo bar'
commands or tracepoint is downloaded).

This patch series is to address the problem that caused by
command 'set foo bar'.  When user types some interesting commands
in console, GDB will send MI notifications about the command
option changes.  The following patch series will address the
problem caused by other aspects in the future.  In this way,
Eclipse is aware of the change of GDB state, and refresh or
update itself accordingly.

Patch 1/6 - 3/6 are about the new observer 'command_option_changed'
and new MI notification '=option-changed' when observer is notified.
Patch 4/6 - 6/6 are about registering three types of command
respectively.  Three commands will send MI notifications if their
option is changed.  There are more than three commands that can
change the state of GDB, but these three are reprsentives of
them.  The rest of commands can be easily changed to sending
MI notifications.

Regression tested on x86_64 native/gdbserver.

[PATCH 1/6] new observer command_option_changed.

[Yao Qi]

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.

gdb:

2012-07-24  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): Notify observer
	command_option_changed if the option of command is changed.

gdb/doc:

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

	* observer.texi: New observer command_option_changed.
---
 gdb/cli/cli-decode.c  |    4 +-
 gdb/cli/cli-decode.h  |    4 ++
 gdb/cli/cli-setshow.c |  142 ++++++++++++++++++++++++++++++++++++++++---------
 gdb/doc/observer.texi |    5 ++
 4 files changed, 129 insertions(+), 26 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index c337b43..6739aef 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 *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..73001a2 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 *auto_boolean_enums[];
 
 #endif /* !defined (CLI_DECODE_H) */
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 7ffb89e..9369b29 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -28,6 +28,8 @@
 #include "cli/cli-cmds.h"
 #include "cli/cli-setshow.h"
 
+#include "observer.h"
+
 /* Prototypes for local functions.  */
 
 static int parse_binary_operation (char *);
@@ -170,50 +172,122 @@ 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;
+
+		if (c->notify_observer_p)
+		  observer_notify_command_option_changed (c->name, new);
+	      }
+	    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);
+
+	      if (c->notify_observer_p)
+		observer_notify_command_option_changed (c->name, arg);
+	    }
 	  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;
+
+		if (c->notify_observer_p)
+		  observer_notify_command_option_changed (c->name, val);
+	      }
+	    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;
+
+		if (c->notify_observer_p)
+		  observer_notify_command_option_changed (c->name,
+							  val ? "on" : "off");
+	      }
+	  }
 	  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;
+
+		if (c->notify_observer_p)
+		  {
+		    char *s = (char *) auto_boolean_enums[val];
+
+		    observer_notify_command_option_changed (c->name, s);
+		  }
+	      }
+	  }
 	  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;
+
+		if (c->notify_observer_p)
+		  {
+		    char s[64];
+
+		    xsnprintf (s, sizeof s, "%d", val);
+		    observer_notify_command_option_changed (c->name, s);
+		  }
+	      }
+	  }
 	  break;
 	case var_integer:
 	case var_zinteger:
@@ -224,11 +298,23 @@ 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;
+
+		if (c->notify_observer_p)
+		  {
+		    char s[64];
+
+		    xsnprintf (s, sizeof s, "%d", val);
+		    observer_notify_command_option_changed (c->name, s);
+		  }
+	      }
 	    break;
 	  }
 	case var_enum:
@@ -293,7 +379,13 @@ 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;
+
+		if (c->notify_observer_p)
+		    observer_notify_command_option_changed (c->name, match);
+	      }
 	  }
 	  break;
 	default:
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index 14d4ac3..a45df36 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -234,6 +234,11 @@ 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{command}, const char *@var{option})
+The option of some @code{set} commands in console are changed.  This method is called after
+a command @code{set command 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.
-- 
1.7.7.6

Re: [PATCH 1/6] new observer command_option_changed.

[Tom Tromey]

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

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

I read through the series to find out why this was needed.
My understanding is that the idea is that options will be segregated
into two kinds: those which report via the observer, and others; and
that the decision is made in the gdb sources.

If that is accurate, I wonder if you could explain the rationale for it.
Another possible design would be to report all option changes; or to let
the MI client choose.

The reason I ask is that making the decision statically seems tricky.
When adding an option, what criteria do we use to decide?  How do we
know what MI users want?  Or what if we want to add a Python hook
here -- maybe we would switch many more options over; but does switching
an option from non-reporting to reporting cause issues for existing MI
clients?  (I assume there is at least a bandwidth concern...)

Yao> +const char *auto_boolean_enums[] = { "on", "off", "auto", NULL };

It seems like now this should be 'const char * const'.
I realize you're just moving it, so normally it would be exempt from
this kind of tweakery, but in this case it is also now exported...

Yao> +		if (c->notify_observer_p)
Yao> +		  {
Yao> +		    char *s = (char *) auto_boolean_enums[val];

I don't think the cast is needed.

Yao> +@deftypefun void command_option_changed (const char *@var{command}, const char *@var{option})
Yao> +The option of some @code{set} commands in console are changed.  This method is called after
Yao> +a command @code{set command option}.
Yao> +@end deftypefun

The lines in the body are all too long.

I think it would be good to describe the meaning of the arguments.


What happens in the case where an option has a validation function that
fails?  IIRC gdb has an internal design flaw here.

Tom

Re: [PATCH 1/6] new observer command_option_changed.

[Yao Qi]

On Tuesday, July 24, 2012 02:38:52 PM Tom Tromey wrote:
> >>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Hi, Tom,
Thanks for the quick review.

> Yao> This patch is to add an extra field 'notify_observer_p' to identify
> Yao> the command will notify observer 'command_option_changed', and teach
> Yao> GDB to check the change of command option, and notify observer.
> 
> I read through the series to find out why this was needed.
> My understanding is that the idea is that options will be segregated
> into two kinds: those which report via the observer, and others; and
> that the decision is made in the gdb sources.
> 
> If that is accurate, I wonder if you could explain the rationale for it.
> Another possible design would be to report all option changes; or to let
> the MI client choose.

The general goal of my work is that "when the internal state of GDB is 
changed, and MI frontend and/or telnet session is not aware of the change, 
notify them to keep their display up to date".  For example, when user types 
command 'set $fp=0x1' in console, GDB should emit a MI notification to 
Eclipse, so that Eclipse can update its "register view" to show the latest 
value of $fp.  When user changes the option "set scheduler-locking step", and 
this change is interesting to Eclipse, GDB should emit a MI notification as 
well.

Not all the changes of GDB state are interesting to MI frontend.  As a subset 
of GDB state changes (other patch series in the future address other GDB state 
changes), not all the command option changes are interesting to MI frontend 
either, so it is not necessary to report all option changes.

> 
> The reason I ask is that making the decision statically seems tricky.
> When adding an option, what criteria do we use to decide?  How do we
> know what MI users want?  Or what if we want to add a Python hook

The criteria is "if Eclipse or other MI frontend needs to know the change of 
this command option, then we should report notification".  IMO, MI user should 
tell us what they want, and we can convert these commands from non-reporting 
to reporting.  Here is a list of commands that Marc Khouzam and I discussed 
offline, and we think they should be switched to reporting.  Note that this is 
not a full list, we may have other commands if needed.

   set circular-trace-buffer
   set disconnected-tracing
   set exec-direction
   set observer
   set may-insert-breakpoints (and the other may-*)
   set record
   set scheduler-locking
   set trace-notes (and other trace-*)
   set auto-solib-add

What do you mean by "add a Python hook"?  Is it something like "using an 
instance of gdb.Command to implement a new GDB CLI command"?  I know few on 
gdb python, but we may add extra parameter NOTIFY in 'Command.__init__', and 
notify command_option_changed observer somewhere.

> here -- maybe we would switch many more options over; but does switching
> an option from non-reporting to reporting cause issues for existing MI
> clients?  (I assume there is at least a bandwidth concern...)

AFAICS, the new MI notification "=option-changed" should not cause issues for 
existing MI clients.  In terms of bandwidth, every "=option-changed" is sent 
once user types command in console, user can't enter many commands at one 
moment (one command per second and ten commands at most, I think).  This 
notification should not occupy much bandwidth.

> What happens in the case where an option has a validation function that
> fails?  IIRC gdb has an internal design flaw here.
> 

If I understand you correctly, "validation function" means cli/cli-
setshow.c:parse_binary_operation and cli/cli-
setshow.c:parse_auto_binary_operation.  When validation fails, an error is 
thrown out, and observer is not called.

-- 
Yao (齐尧)

Re: [PATCH 1/6] new observer command_option_changed.

[Tom Tromey]

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

Yao> The general goal of my work is that "when the internal state of GDB
Yao> is changed, and MI frontend and/or telnet session is not aware of
Yao> the change, notify them to keep their display up to date".

I'm fully on board with this plan.

Yao> What do you mean by "add a Python hook"?

Sorry, I wasn't clear.

Suppose we want to add a new Python event so that Python code can also
detect parameter changes.  My supposition is that in this case we'd want
to not filter, and just watch them all.

Yao> In terms of bandwidth, every "=option-changed" is sent once user
Yao> types command in console, user can't enter many commands at one
Yao> moment (one command per second and ten commands at most, I think).
Yao> This notification should not occupy much bandwidth.

In that case, why not just report them all?
I agree with your thinking here, that the rate is going to be quite
low.  I'm not sure why I didn't realize that yesterday :)

Reporting them all means simpler code on the gdb side and also lets us
avoid the new set/show constructors.  I don't see a downside.

Tom> What happens in the case where an option has a validation function that
Tom> fails?  IIRC gdb has an internal design flaw here.

Yao> If I understand you correctly, "validation function" means cli/cli-
Yao> setshow.c:parse_binary_operation and cli/cli-
Yao> setshow.c:parse_auto_binary_operation.  When validation fails, an error is 
Yao> thrown out, and observer is not called.

Sorry again for the lack of clarity.

I mean the call to c->func at the end of do_setshow_command.

There are various spots in gdb that use a hidden variable to keep the
"real" setting in case setting the value here fails.

For example, try "set input-radix 1".  This is an error, but I think
with your patch the MI client will see a notification saying that it was
changed to "1".

The design flaw here is that "set" functions do their validation after
the setting has been made, not before.

One approach to this would be to fix this design flaw.  That's likely to
be a lot of work...

Another approach might be to move the observer notification later.

Tom

Re: [PATCH 1/6] new observer command_option_changed.

[Pedro Alves]

On 07/25/2012 03:43 PM, Tom Tromey wrote:
>>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
> 
> Yao> The general goal of my work is that "when the internal state of GDB
> Yao> is changed, and MI frontend and/or telnet session is not aware of
> Yao> the change, notify them to keep their display up to date".
> 
> I'm fully on board with this plan.
> 
> Yao> What do you mean by "add a Python hook"?
> 
> Sorry, I wasn't clear.
> 
> Suppose we want to add a new Python event so that Python code can also
> detect parameter changes.  My supposition is that in this case we'd want
> to not filter, and just watch them all.
> 
> Yao> In terms of bandwidth, every "=option-changed" is sent once user
> Yao> types command in console, user can't enter many commands at one
> Yao> moment (one command per second and ten commands at most, I think).
> Yao> This notification should not occupy much bandwidth.
> 
> In that case, why not just report them all?
> I agree with your thinking here, that the rate is going to be quite
> low.  I'm not sure why I didn't realize that yesterday :)

The case where rate would be a bit higher is on canned sequences of
commands, like user defined functions, that could do things like
wrap a command with a set foo X; something; set foo Y;

> Reporting them all means simpler code on the gdb side and also lets us
> avoid the new set/show constructors.  I don't see a downside.

I agree.  Statically deciding which options to report changes to will
not scale.  It's about the same as deferring the reporting all
changes to a gdb of 10 years from now, when enough frontends have
requested we notify more and more options.  :-)  It also essentially
is a one way street - only you report a changes for an option, you'll
never decide to stop reporting it, for fear of that breaking some
frontend.  Best is just to either:

- report all
- report none, but instead report a single "some option changed", and
  let the frontend refresh all its state.
- let the frontend tell gdb which settings it is interested in.

The "all" option seems the simplest.

> Tom> What happens in the case where an option has a validation function that
> Tom> fails?  IIRC gdb has an internal design flaw here.
> 
> Yao> If I understand you correctly, "validation function" means cli/cli-
> Yao> setshow.c:parse_binary_operation and cli/cli-
> Yao> setshow.c:parse_auto_binary_operation.  When validation fails, an error is 
> Yao> thrown out, and observer is not called.
> 
> Sorry again for the lack of clarity.
> 
> I mean the call to c->func at the end of do_setshow_command.
> 
> There are various spots in gdb that use a hidden variable to keep the
> "real" setting in case setting the value here fails.
> 
> For example, try "set input-radix 1".  This is an error, but I think
> with your patch the MI client will see a notification saying that it was
> changed to "1".
> 
> The design flaw here is that "set" functions do their validation after
> the setting has been made, not before.

Yeah, or rather, that there is not "validate" hook, and the "set" hook
is abused for that.

> One approach to this would be to fix this design flaw.  That's likely to
> be a lot of work...

I don't think there any that many cases to fix.

> Another approach might be to move the observer notification later.

-- 
Pedro Alves

Re: [PATCH 1/6] new observer command_option_changed.

[Tom Tromey]

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

Yao> +	      if (c->notify_observer_p)
Yao> +		observer_notify_command_option_changed (c->name, arg);

Last night I realized that this does the wrong thing for multi-word
parameters.  E.g., for "set print elements", the MI client will just see
"elements".

Tom

Re: [PATCH 1/6] new observer command_option_changed.

[Yao Qi]

On Wednesday, July 25, 2012 08:31:55 AM Tom Tromey wrote:
> Yao> +        if (c->notify_observer_p)
> Yao> +          observer_notify_command_option_changed (c->name, arg);
> 
> Last night I realized that this does the wrong thing for multi-word
> parameters.  E.g., for "set print elements", the MI client will just see
> "elements".

Right, we are unable to get the previous command instance "print" in command 
instance  "elements".  So we have to move 
'observer_notify_command_option_changed' out of do_setshow_command to its 
caller, top.c:execute_command, where the full command option and value is 
available.  Then, we can change 'do_setshow_command' to return a boolean value 
indicating option is changed or not, and call 
observer_notify_command_option_changed if do_setshow_command returns true.

It works fine for me now.  I'll post a patch.

-- 
Yao (齐尧)