Re: RFA: fix PR 1815

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> There is a difference, however, in the fact that hooks are lost
Joel> when redifining a command.  Your patch handles this case much better.
Joel> I'll open a PR for this problem, and hopefully someone will find the
Joel> time to fix it.

Thanks.

>> -void
>> +static struct cmd_list_element *

Joel> Can you update the command to mention the fact that the function
Joel> returns the list of aliases to the command being deleted?

Done.

Joel> I was wondering if this is something that can be implemented
Joel> with gdb_test_multiple instead of using gdb_expect/send_gdb.

I thought I tried this, but I redid it now and the appended seems to
work.

Built & regtested on x86-64 (compile farm).
Ok?

Tom

2008-12-09  Tom Tromey  <tromey@redhat.com>

	PR gdb/1815:
	* cli/cli-decode.c (delete_cmd): Forward declare.
	(delete_cmd): Now static.  Change return type.  Remove command
	from alias chain.  Rewrite.
	(add_cmd): Initialize new fields.  Update cmd_pointer on all
	aliases.
	(add_alias_cmd): Put command on alias chain.
	* command.h (delete_cmd): Don't declare.
	* cli/cli-decode.h (delete_cmd): Don't declare.
	(struct cmd_list_element) <aliases, alias_chain>: New fields.

2008-12-09  Tom Tromey  <tromey@redhat.com>

	* gdb.base/commands.exp (redefine_backtrace_test): New proc.
	Call it.

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 9cc52ba..613b8d7 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -37,6 +37,9 @@
 
 static void undef_cmd_error (char *, char *);
 
+static struct cmd_list_element *delete_cmd (char *name,
+					    struct cmd_list_element **list);
+
 static struct cmd_list_element *find_cmd (char *command,
 					  int len,
 					  struct cmd_list_element *clist,
@@ -154,9 +157,13 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
 {
   struct cmd_list_element *c
   = (struct cmd_list_element *) xmalloc (sizeof (struct cmd_list_element));
-  struct cmd_list_element *p;
+  struct cmd_list_element *p, *iter;
 
-  delete_cmd (name, list);
+  /* Turn each alias of the old command into an alias of the new
+     command.  */
+  c->aliases = delete_cmd (name, list);
+  for (iter = c->aliases; iter; iter = iter->alias_chain)
+    iter->cmd_pointer = c;
 
   if (*list == NULL || strcmp ((*list)->name, name) >= 0)
     {
@@ -198,6 +205,7 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
   c->hookee_pre = NULL;
   c->hookee_post = NULL;
   c->cmd_pointer = NULL;
+  c->alias_chain = NULL;
 
   return c;
 }
@@ -239,7 +247,9 @@ add_alias_cmd (char *name, char *oldname, enum command_class class,
 
   if (old == 0)
     {
-      delete_cmd (name, list);
+      struct cmd_list_element *aliases = delete_cmd (name, list);
+      /* If this happens, it means a programmer error somewhere.  */
+      gdb_assert (!aliases);
       return 0;
     }
 
@@ -252,6 +262,8 @@ add_alias_cmd (char *name, char *oldname, enum command_class class,
   c->allow_unknown = old->allow_unknown;
   c->abbrev_flag = abbrev_flag;
   c->cmd_pointer = old;
+  c->alias_chain = old->aliases;
+  old->aliases = c;
   return c;
 }
 
@@ -614,42 +626,58 @@ add_setshow_zinteger_cmd (char *name, enum command_class class,
 			NULL, NULL);
 }
 
-/* Remove the command named NAME from the command list.  */
+/* Remove the command named NAME from the command list.  Return the
+   list commands which were aliased to the deleted command.  If the
+   command had no aliases, return NULL.  */
 
-void
+static struct cmd_list_element *
 delete_cmd (char *name, struct cmd_list_element **list)
 {
-  struct cmd_list_element *c;
-  struct cmd_list_element *p;
+  struct cmd_list_element *iter;
+  struct cmd_list_element **previous_chain_ptr;
+  struct cmd_list_element *aliases = NULL;
+
+  previous_chain_ptr = list;
 
-  while (*list && strcmp ((*list)->name, name) == 0)
+  for (iter = *previous_chain_ptr; iter; iter = *previous_chain_ptr)
     {
-      if ((*list)->hookee_pre)
-      (*list)->hookee_pre->hook_pre = 0;   /* Hook slips out of its mouth */
-      if ((*list)->hookee_post)
-      (*list)->hookee_post->hook_post = 0; /* Hook slips out of its bottom  */
-      p = (*list)->next;
-      xfree (* list);
-      *list = p;
+      if (strcmp (iter->name, name) == 0)
+	{
+	  if (iter->hookee_pre)
+	    iter->hookee_pre->hook_pre = 0;
+	  if (iter->hookee_post)
+	    iter->hookee_post->hook_post = 0;
+
+	  /* Update the link.  */
+	  *previous_chain_ptr = iter->next;
+
+	  aliases = iter->aliases;
+
+	  /* If this command was an alias, remove it from the list of
+	     aliases.  */
+	  if (iter->cmd_pointer)
+	    {
+	      struct cmd_list_element **prevp = &iter->cmd_pointer->aliases;
+	      struct cmd_list_element *a = *prevp;
+
+	      while (a != iter)
+		{
+		  prevp = &a->alias_chain;
+		  a = *prevp;
+		}
+	      *prevp = iter->alias_chain;
+	    }
+
+	  xfree (iter);
+
+	  /* We won't see another command with the same name.  */
+	  break;
+	}
+      else
+	previous_chain_ptr = &iter->next;
     }
 
-  if (*list)
-    for (c = *list; c->next;)
-      {
-	if (strcmp (c->next->name, name) == 0)
-	  {
-          if (c->next->hookee_pre)
-            c->next->hookee_pre->hook_pre = 0; /* hooked cmd gets away.  */
-          if (c->next->hookee_post)
-            c->next->hookee_post->hook_post = 0; /* remove post hook */
-                                               /* :( no fishing metaphore */
-	    p = c->next->next;
-	    xfree (c->next);
-	    c->next = p;
-	  }
-	else
-	  c = c->next;
-      }
+  return aliases;
 }
 
 /* Shorthands to the commands above. */
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 4e2dbc1..9d27f0f 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -197,6 +197,12 @@ struct cmd_list_element
     /* Pointer to command that is aliased by this one, so the
        aliased command can be located in case it has been hooked.  */
     struct cmd_list_element *cmd_pointer;
+
+    /* Start of a linked list of all aliases of this command.  */
+    struct cmd_list_element *aliases;
+
+    /* Link pointer for aliases on an alias list.  */
+    struct cmd_list_element *alias_chain;
   };
 
 /* API to the manipulation of command lists.  */
@@ -290,8 +296,6 @@ extern char **complete_on_cmdlist (struct cmd_list_element *, char *, char *);
 
 extern char **complete_on_enum (const char *enumlist[], char *, char *);
 
-extern void delete_cmd (char *, struct cmd_list_element **);
-
 extern void help_cmd_list (struct cmd_list_element *, enum command_class,
 			   char *, int, struct ui_file *);
 
diff --git a/gdb/command.h b/gdb/command.h
index cf09c01..5678c9e 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -193,8 +193,6 @@ extern char **complete_on_cmdlist (struct cmd_list_element *, char *, char *);
 
 extern char **complete_on_enum (const char *enumlist[], char *, char *);
 
-extern void delete_cmd (char *, struct cmd_list_element **);
-
 extern void help_cmd (char *, struct ui_file *);
 
 extern void help_list (struct cmd_list_element *, char *,
diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 301b995..7da3f23 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -692,6 +692,33 @@ proc if_commands_test {} {
     }
 }
 
+proc redefine_backtrace_test {} {
+    global gdb_prompt
+
+    gdb_test_multiple "define backtrace" "define backtrace" {
+	-re "Really redefine built-in.*$" {
+	    send_gdb "y\n"
+	}
+
+	-re "End with"  {
+	    pass "define backtrace in redefine_backtrace_test"
+	}
+        default {
+	    fail "(timeout or eof) define backtrace in redefine_backtrace_test"
+	}
+    }
+    gdb_test "echo hibob\\n\nend" \
+	    "" \
+	    "enter commands in redefine_backtrace_test"
+
+    gdb_test "backtrace" \
+	    "hibob" \
+	    "execute backtrace command in redefine_backtrace_test"
+    gdb_test "bt" \
+	    "hibob" \
+	    "execute bt command in redefine_backtrace_test"
+}
+
 gdbvar_simple_if_test
 gdbvar_simple_while_test
 gdbvar_complex_if_while_test
@@ -710,3 +737,5 @@ temporary_breakpoint_commands
 stray_arg0_test
 recursive_source_test
 if_commands_test
+# This one should come last, as it redefines "backtrace".
+redefine_backtrace_test