This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/3] new observer command_param_changed.


On Monday, August 06, 2012 02:20:47 PM Tom Tromey wrote:
> Yao>      {
> Yao> +      char *name;
> Yao> +
> Yao> +      /* Compute the whole multi-word command options.  If user types
> command Yao> +   'set foo bar baz on', c->name is 'baz', and GDB can't pass
> "bar" to Yao> +   command option change notification, because it is
> confusing.  We can Yao> +   trace back through field 'prefix' to compute
> the whole options, Yao> +   and pass "foo bar baz" to notification.  */
> Yao> +      {
> 
> Two blocks separated only by a declaration seems unusual.
> I would suggest just a single block.

OK, fixed.

> 
> Yao> +  /* Skip the command 'set' in CMDS.  */
> Yao> +  i--;
> Yao> +  gdb_assert (cmds[i]->prefixlist == &setlist);
> 
> I wonder if this can trigger somehow.  For example, if a Python
> programmer makes a Parameter under 'maint set' that is in an unexpected
> command class.

Yes, this case will trigger this assertion failure here.

> I think it would be safer perhaps to just avoid the notification if the
> prefixlist is not setlist.

OK, replace the assertion with the following condition check.

+      /* Don't trigger any observer notification if prefixlist is not
+        setlist.  */
+      i--;
+      if (cmds[i]->prefixlist != &setlist)
+       {
+         xfree (cmds);
+         xfree (name);
+
+         return;
+       }

The rest is unchanged.

-- 
Yao (éå)

gdb:

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

	* cli/cli-decode.c (set_cmd_prefix): New.
	(lookup_cmd_for_prefixlist): New.
	(add_prefix_cmd): Call set_cmd_prefix and update field 'prefix'
	of each cmd_list_element in *prefixlist.
	(add_setshow_cmd_full): set_cmd_prefix.
	(add_alias_cmd): Likewise.
	* cli/cli-decode.h (struct cmd_list_element) <prefix>: New field.
	Declare 'auto_boolean_enums'.
	* cli/cli-setshow.c: Include "observer.h".
	(notify_command_param_changed_p): New.
	(add_setshow_auto_boolean_cmd): Move auto_boolean_enums out.
	Remove 'static'.
	(do_setshow_command): Split it to ...
	(do_set_command, do_show_command): ... them.  New.
	(do_set_command): Call observer_notify_command_param_changed if
	notify_command_param_changed_p returns true.
	(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 do_show_command.

gdb/doc:

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

	* observer.texi: New observer command_param_changed.
---
 gdb/auto-load.c       |    2 +-
 gdb/cli/cli-decode.c  |   67 ++++++++++++-
 gdb/cli/cli-decode.h  |    4 +
 gdb/cli/cli-setshow.c |  274 ++++++++++++++++++++++++++++++++++++++++++-------
 gdb/cli/cli-setshow.h |   10 +-
 gdb/doc/observer.texi |    8 ++
 gdb/remote.c          |    2 +-
 gdb/top.c             |    6 +-
 8 files changed, 324 insertions(+), 49 deletions(-)

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 2cc52c6..03a7539 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -1022,7 +1022,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..3c2e152 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -52,6 +52,53 @@ static struct cmd_list_element *find_cmd (char *command,
 
 static void help_all (struct ui_file *stream);
 
+/* Look up a command whose 'prefixlist' is KEY.  Return the command if found,
+   otherwise return NULL.  */
+
+static struct cmd_list_element *
+lookup_cmd_for_prefixlist (struct cmd_list_element **key,
+			   struct cmd_list_element *list)
+{
+  struct cmd_list_element *p = NULL;
+
+  for (p = list; p != NULL; p = p->next)
+    {
+      struct cmd_list_element *q;
+
+      if (p->prefixlist == NULL)
+	continue;
+      else if (p->prefixlist == key)
+	return p;
+
+      q = lookup_cmd_for_prefixlist (key, *(p->prefixlist));
+      if (q != NULL)
+	return q;
+    }
+
+  return NULL;
+}
+
+static void
+set_cmd_prefix (struct cmd_list_element *c, struct cmd_list_element **list)
+{
+  struct cmd_list_element *p;
+
+  /* Check to see if *LIST contains any element other than C.  */
+  for (p = *list; p != NULL; p = p->next)
+    if (p != c)
+      break;
+
+  if (p == NULL)
+    {
+      /* *SET_LIST only contains SET.  */
+      p = lookup_cmd_for_prefixlist (list, setlist);
+
+      c->prefix = p ? (p->cmd_pointer ? p->cmd_pointer : p) : p;
+    }
+  else
+    c->prefix = p->prefix;
+}
+
 static void
 print_help_for_command (struct cmd_list_element *c, char *prefix, int recurse,
 			struct ui_file *stream);
@@ -193,6 +240,7 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
   c->prefixlist = NULL;
   c->prefixname = NULL;
   c->allow_unknown = 0;
+  c->prefix = NULL;
   c->abbrev_flag = 0;
   set_cmd_completer (c, make_symbol_completion_list_fn);
   c->destroyer = NULL;
@@ -268,6 +316,8 @@ add_alias_cmd (char *name, char *oldname, enum command_class class,
   c->cmd_pointer = old;
   c->alias_chain = old->aliases;
   old->aliases = c;
+
+  set_cmd_prefix (c, list);
   return c;
 }
 
@@ -284,10 +334,21 @@ add_prefix_cmd (char *name, enum command_class class,
 		struct cmd_list_element **list)
 {
   struct cmd_list_element *c = add_cmd (name, class, fun, doc, list);
+  struct cmd_list_element *p;
 
   c->prefixlist = prefixlist;
   c->prefixname = prefixname;
   c->allow_unknown = allow_unknown;
+
+  if (list == &cmdlist)
+    c->prefix = NULL;
+  else
+    set_cmd_prefix (c, list);
+
+  /* Update the field 'prefix' of each cmd_list_element in *PREFIXLIST.  */
+  for (p = *prefixlist; p != NULL; p = p->next)
+    p->prefix = c;
+
   return c;
 }
 
@@ -392,6 +453,9 @@ add_setshow_cmd_full (char *name,
 			     full_set_doc, set_list);
   if (set_func != NULL)
     set_cmd_sfunc (set, set_func);
+
+  set_cmd_prefix (set, set_list);
+
   show = add_set_or_show_cmd (name, show_cmd, class, var_type, var,
 			      full_show_doc, show_list);
   show->show_value_func = show_func;
@@ -430,6 +494,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 +511,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..edae6e8 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -149,6 +149,9 @@ struct cmd_list_element
        recognized; call the prefix's own function in that case.  */
     char allow_unknown;
 
+    /* The prefix command of this command.  */
+    struct cmd_list_element *prefix;
+
     /* Nonzero says this is an abbreviation, and should not
        be mentioned in lists of commands.
        This allows "br<tab>" to complete to "break", which it
@@ -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..d0cf211 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -21,6 +21,7 @@
 #include <ctype.h>
 #include "gdb_string.h"
 #include "arch-utils.h"
+#include "observer.h"
 
 #include "ui-out.h"
 
@@ -32,6 +33,21 @@
 
 static int parse_binary_operation (char *);
 
+/* Return true if the change of command parameter should be notified.  */
+
+static int
+notify_command_param_changed_p (int param_changed, struct cmd_list_element *c)
+{
+  if (param_changed == 0)
+    return 0;
+
+  if (c->class == class_maintenance || c->class == class_deprecated
+      || c->class == class_obscure)
+    return 0;
+
+  return 1;
+}
+
 
 static enum auto_boolean
 parse_auto_binary_operation (const char *arg)
@@ -116,18 +132,19 @@ 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.  */
 
 void
-do_setshow_command (char *arg, int from_tty, struct cmd_list_element *c)
+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 +187,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 +297,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 +372,137 @@ 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);
+
+  if (notify_command_param_changed_p (option_changed, c))
     {
+      char *name, *cp;
+      struct cmd_list_element **cmds;
+      struct cmd_list_element *p;
+      int i;
+      int length = 0;
+
+      /* Compute the whole multi-word command options.  If user types command
+	 'set foo bar baz on', c->name is 'baz', and GDB can't pass "bar" to
+	 command option change notification, because it is confusing.  We can
+	 trace back through field 'prefix' to compute the whole options,
+	 and pass "foo bar baz" to notification.  */
+
+      for (i = 0, p = c; p != NULL; i++)
+	{
+	  length += strlen (p->name);
+	  length++;
+
+	  p = p->prefix;
+	}
+      cp = name = xmalloc (length);
+      cmds = xmalloc (sizeof (struct cmd_list_element *) * i);
+
+      /* Track back through filed 'prefix' and cache them in CMDS.  */
+      for (i = 0, p = c; p != NULL; i++)
+	{
+	  cmds[i] = p;
+	  p = p->prefix;
+	}
+
+      /* Don't trigger any observer notification if prefixlist is not
+	 setlist.  */
+      i--;
+      if (cmds[i]->prefixlist != &setlist)
+	{
+	  xfree (cmds);
+	  xfree (name);
+
+	  return;
+	}
+      /* Traverse them in the reversed order, and copy their names into
+	 NAME.  */
+      for (i--; i >= 0; i--)
+	{
+	  memcpy (cp, cmds[i]->name, strlen (cmds[i]->name));
+	  cp += strlen (cmds[i]->name);
+
+	  if (i != 0)
+	    {
+	      cp[0] = ' ';
+	      cp++;
+	    }
+	}
+      cp[0] = 0;
+
+      xfree (cmds);
+
+      switch (c->var_type)
+	{
+	case var_string:
+	case var_string_noescape:
+	case var_filename:
+	case var_optional_filename:
+	case var_enum:
+	  observer_notify_command_param_changed (name, *(char **) c->var);
+	  break;
+	case var_boolean:
+	  {
+	    char *opt = *(int *) c->var ? "on" : "off";
+
+	    observer_notify_command_param_changed (name, opt);
+	  }
+	  break;
+	case var_auto_boolean:
+	  {
+	    const char *s = auto_boolean_enums[*(enum auto_boolean *) c->var];
+
+	    observer_notify_command_param_changed (name, s);
+	  }
+	  break;
+	case var_uinteger:
+	case var_zuinteger:
+	  {
+	    char s[64];
+
+	    xsnprintf (s, sizeof s, "%u", *(unsigned int *) c->var);
+	    observer_notify_command_param_changed (name, s);
+	  }
+	  break;
+	case var_integer:
+	case var_zinteger:
+	  {
+	    char s[64];
+
+	    xsnprintf (s, sizeof s, "%d", *(int *) c->var);
+	    observer_notify_command_param_changed (name, s);
+	  }
+	  break;
+	}
+      xfree (name);
+    }
+}
+
+/* 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 +588,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 +629,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..ffe9abd 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 void 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..bf84820 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_param_changed (const char *@var{param}, const char *@var{value})
+The parameter of some @code{set} commands in console are changed.  This
+method is called after a command @code{set @var{param} @var{value}}.
+@var{param} is the parameter of @code{set} command, and @var{value}
+is the value of changed parameter.
+
+@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 213c68c..8251d1b 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -474,8 +474,10 @@ execute_command (char *p, int from_tty)
       /* 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)
+	do_set_command (arg, from_tty, c);
+      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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]