RFA: fix PR cli/2563

RFA: fix PR cli/2563

[Tom Tromey]

This patch fixes PR cli/2563.

Joel pointed this out in response to my fix for PR 1815.  The bug is
that redefining a hooked command does not preserve the hook.

The fix is to apply the hooks from the old command to the new one when
redefining a command.

Built and regtested on x86-64 (compile farm).
New test case included.

Please review.

Tom

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

	PR cli/2563:
	* cli/cli-decode.c (delete_cmd): Add hook-related out parameters.
	(add_cmd): Update.  Install hooks into new command.
	(add_alias_cmd): Update.

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

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

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 613b8d7..f1ef680 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -38,7 +38,11 @@
 static void undef_cmd_error (char *, char *);
 
 static struct cmd_list_element *delete_cmd (char *name,
-					    struct cmd_list_element **list);
+					    struct cmd_list_element **list,
+					    struct cmd_list_element **prehook,
+					    struct cmd_list_element **prehookee,
+					    struct cmd_list_element **posthook,
+					    struct cmd_list_element **posthookee);
 
 static struct cmd_list_element *find_cmd (char *command,
 					  int len,
@@ -161,9 +165,18 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
 
   /* Turn each alias of the old command into an alias of the new
      command.  */
-  c->aliases = delete_cmd (name, list);
+  c->aliases = delete_cmd (name, list, &c->hook_pre, &c->hookee_pre,
+			   &c->hook_post, &c->hookee_post);
   for (iter = c->aliases; iter; iter = iter->alias_chain)
     iter->cmd_pointer = c;
+  if (c->hook_pre)
+    c->hook_pre->hookee_pre = c;
+  if (c->hookee_pre)
+    c->hookee_pre->hook_pre = c;
+  if (c->hook_post)
+    c->hook_post->hookee_post = c;
+  if (c->hookee_post)
+    c->hookee_post->hook_post = c;
 
   if (*list == NULL || strcmp ((*list)->name, name) >= 0)
     {
@@ -189,8 +202,6 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
   c->flags = 0;
   c->replacement = NULL;
   c->pre_show_hook = NULL;
-  c->hook_pre  = NULL;
-  c->hook_post = NULL;
   c->hook_in = 0;
   c->prefixlist = NULL;
   c->prefixname = NULL;
@@ -202,8 +213,6 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
   c->var_type = var_boolean;
   c->enums = NULL;
   c->user_commands = NULL;
-  c->hookee_pre = NULL;
-  c->hookee_post = NULL;
   c->cmd_pointer = NULL;
   c->alias_chain = NULL;
 
@@ -247,9 +256,13 @@ add_alias_cmd (char *name, char *oldname, enum command_class class,
 
   if (old == 0)
     {
-      struct cmd_list_element *aliases = delete_cmd (name, list);
+      struct cmd_list_element *prehook, *prehookee, *posthook, *posthookee;
+      struct cmd_list_element *aliases = delete_cmd (name, list,
+						     &prehook, &prehookee,
+						     &posthook, &posthookee);
       /* If this happens, it means a programmer error somewhere.  */
-      gdb_assert (!aliases);
+      gdb_assert (!aliases && !prehook && prehookee
+		  && !posthook && ! posthookee);
       return 0;
     }
 
@@ -628,15 +641,26 @@ add_setshow_zinteger_cmd (char *name, enum command_class class,
 
 /* 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.  */
+   command had no aliases, return NULL.  The various *HOOKs are set to
+   the pre- and post-hook commands for the deleted command.  If the
+   command does not have a hook, the corresponding out parameter is
+   set to NULL.  */
 
 static struct cmd_list_element *
-delete_cmd (char *name, struct cmd_list_element **list)
+delete_cmd (char *name, struct cmd_list_element **list,
+	    struct cmd_list_element **prehook,
+	    struct cmd_list_element **prehookee,
+	    struct cmd_list_element **posthook,
+	    struct cmd_list_element **posthookee)
 {
   struct cmd_list_element *iter;
   struct cmd_list_element **previous_chain_ptr;
   struct cmd_list_element *aliases = NULL;
 
+  *prehook = NULL;
+  *prehookee = NULL;
+  *posthook = NULL;
+  *posthookee = NULL;
   previous_chain_ptr = list;
 
   for (iter = *previous_chain_ptr; iter; iter = *previous_chain_ptr)
@@ -645,8 +669,12 @@ delete_cmd (char *name, struct cmd_list_element **list)
 	{
 	  if (iter->hookee_pre)
 	    iter->hookee_pre->hook_pre = 0;
+	  *prehook = iter->hook_pre;
+	  *prehookee = iter->hookee_pre;
 	  if (iter->hookee_post)
 	    iter->hookee_post->hook_post = 0;
+	  *posthook = iter->hook_post;
+	  *posthookee = iter->hookee_post;
 
 	  /* Update the link.  */
 	  *previous_chain_ptr = iter->next;
diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index f6be3ea..f3dd9ac 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -692,6 +692,40 @@ proc if_commands_test {} {
     }
 }
 
+proc redefine_hook_test {} {
+    global gdb_prompt
+
+    gdb_test "define one\nend" \
+      "" \
+      "define one"
+
+    gdb_test "define hook-one\necho hibob\\n\nend" \
+      "" \
+      "define hook-one"
+
+    gdb_test_multiple "define one" "redefine one" {
+	-re "Redefine command .one.. .y or n. $" {
+	    send_gdb "y\n"
+	    exp_continue
+	}
+
+	-re "End with"  {
+	    pass "define one in redefine_hook_test"
+	}
+        default {
+	    fail "(timeout or eof) define one in redefine_hook_test"
+	}
+    }
+
+    gdb_test "end" \
+	    "" \
+	    "enter commands for one redefinition in redefine_hook_test"
+
+    gdb_test "one" \
+	    "hibob" \
+	    "execute one command in redefine_hook_test"
+}
+
 proc redefine_backtrace_test {} {
     global gdb_prompt
 
@@ -738,5 +772,6 @@ temporary_breakpoint_commands
 stray_arg0_test
 recursive_source_test
 if_commands_test
+redefine_hook_test
 # This one should come last, as it redefines "backtrace".
 redefine_backtrace_test

Re: RFA: fix PR cli/2563

[Joel Brobecker]

> Joel pointed this out in response to my fix for PR 1815.  The bug is
> that redefining a hooked command does not preserve the hook.

Thanks for fixing this bug, Tom!

> 2008-12-11  Tom Tromey  <tromey@redhat.com>
> 
> 	PR cli/2563:
> 	* cli/cli-decode.c (delete_cmd): Add hook-related out parameters.
> 	(add_cmd): Update.  Install hooks into new command.
> 	(add_alias_cmd): Update.
> 
> 2008-12-11  Tom Tromey  <tromey@redhat.com>
> 
> 	* gdb.base/commands.exp (redefine_hook_test): New proc.
> 	Call it.

This is OK.

Thank you,
-- 
Joel

Re: RFA: fix PR cli/2563

[Tom Tromey]

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

>> Joel pointed this out in response to my fix for PR 1815.  The bug is
>> that redefining a hooked command does not preserve the hook.

Joel> Thanks for fixing this bug, Tom!

You're welcome :)

>> 2008-12-11  Tom Tromey  <tromey@redhat.com>
>> PR cli/2563:
[...]

Joel> This is OK.

Committed.

Tom