[PATCH] tui: replace deprecated_register_changed_hook with observer

[PATCH] tui: replace deprecated_register_changed_hook with observer

[Patrick Palka]

This is a straightforward replacement of the TUI's use of the
aforementioned hook with the register_changed observer.  Since this was
the only user of the hook, this patch also removes the hook.

[ I am not sure if the changes to the function tui_register_changed are
  correct.  In particular, the inputted frame argument is now passed down
  to tui_check_data_values instead of the frame returned by
  get_selected_frame.  The frame argument passed to each register_changed
  observer corresponds to the VALUE_FRAME_ID of the register being
  modified within a register assignment, e.g. the $rax in "print $rax =
  FOO".  When would the frame corresponding to the VALUE_FRAME_ID of a
  register not be the currently selected frame?  ]

gdb/ChangeLog:

	* defs.h (deprecated_register_changed_hook): Remove prototype.
	* interps.c (clear_iterpreter_hooks): Remove reference to
	deprecated_register_changed_hook.
	* top.c (deprecated_register_changed_hook): Remove prototype.
	* valops.c (value_assign): Remove reference to
	deprecated_register_changed_hook.
	* tui/tui-hooks.c (tui_register_changed): Add parameter "frame".
	Use it instead of calling get_selected_frame.
	(tui_register_changed_observer): Define.
	(tui_install_hooks): Remove reference to
	deprecated_register_changed_hook.  Set
	tui_register_changed_observer.
	(tui_remove_hooks): Remove reference to
	deprecated_register_changed_hook.  Unset
	tui_register_changed_observer.
---
 gdb/defs.h          |  1 -
 gdb/interps.c       |  1 -
 gdb/top.c           |  5 -----
 gdb/tui/tui-hooks.c | 16 +++++++---------
 gdb/valops.c        |  2 --
 5 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/gdb/defs.h b/gdb/defs.h
index 32b08bb..a555da1 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -649,7 +649,6 @@ extern void (*deprecated_readline_begin_hook) (char *, ...)
      ATTRIBUTE_FPTR_PRINTF_1;
 extern char *(*deprecated_readline_hook) (const char *);
 extern void (*deprecated_readline_end_hook) (void);
-extern void (*deprecated_register_changed_hook) (int regno);
 extern void (*deprecated_context_hook) (int);
 extern ptid_t (*deprecated_target_wait_hook) (ptid_t ptid,
 					      struct target_waitstatus *status,
diff --git a/gdb/interps.c b/gdb/interps.c
index 4c1e6cc..d825e14 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -370,7 +370,6 @@ clear_interpreter_hooks (void)
   deprecated_readline_begin_hook = 0;
   deprecated_readline_hook = 0;
   deprecated_readline_end_hook = 0;
-  deprecated_register_changed_hook = 0;
   deprecated_context_hook = 0;
   deprecated_target_wait_hook = 0;
   deprecated_call_command_hook = 0;
diff --git a/gdb/top.c b/gdb/top.c
index 01fddd2..1e30b1c 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -221,11 +221,6 @@ void (*deprecated_detach_hook) (void);
 
 void (*deprecated_interactive_hook) (void);
 
-/* Tell the GUI someone changed the register REGNO.  -1 means
-   that the caller does not know which register changed or
-   that several registers have changed (see value_assign).  */
-void (*deprecated_register_changed_hook) (int regno);
-
 /* Called when going to wait for the target.  Usually allows the GUI
    to run while waiting for target events.  */
 
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 0eb2f07..ccf0989 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -67,15 +67,12 @@ tui_new_objfile_hook (struct objfile* objfile)
 static int tui_refreshing_registers = 0;
 
 static void
-tui_register_changed_hook (int regno)
+tui_register_changed (struct frame_info *frame, int regno)
 {
-  struct frame_info *fi;
-
-  fi = get_selected_frame (NULL);
   if (tui_refreshing_registers == 0)
     {
       tui_refreshing_registers = 1;
-      tui_check_data_values (fi);
+      tui_check_data_values (frame);
       tui_refreshing_registers = 0;
     }
 }
@@ -226,6 +223,7 @@ static struct observer *tui_inferior_exit_observer;
 static struct observer *tui_about_to_proceed_observer;
 static struct observer *tui_before_prompt_observer;
 static struct observer *tui_normal_stop_observer;
+static struct observer *tui_register_changed_observer;
 
 /* Install the TUI specific hooks.  */
 void
@@ -253,8 +251,8 @@ tui_install_hooks (void)
     = observer_attach_before_prompt (tui_before_prompt);
   tui_normal_stop_observer
     = observer_attach_normal_stop (tui_normal_stop);
-
-  deprecated_register_changed_hook = tui_register_changed_hook;
+  tui_register_changed_observer
+    = observer_attach_register_changed (tui_register_changed);
 }
 
 /* Remove the TUI specific hooks.  */
@@ -263,8 +261,6 @@ tui_remove_hooks (void)
 {
   deprecated_print_frame_info_listing_hook = 0;
   deprecated_query_hook = 0;
-  deprecated_register_changed_hook = 0;
-
   /* Remove our observers.  */
   observer_detach_breakpoint_created (tui_bp_created_observer);
   tui_bp_created_observer = NULL;
@@ -280,6 +276,8 @@ tui_remove_hooks (void)
   tui_before_prompt_observer = NULL;
   observer_detach_normal_stop (tui_normal_stop_observer);
   tui_normal_stop_observer = NULL;
+  observer_detach_register_changed (tui_register_changed_observer);
+  tui_register_changed_observer = NULL;
 }
 
 void _initialize_tui_hooks (void);
diff --git a/gdb/valops.c b/gdb/valops.c
index 66c63c1..c4ff032 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1170,8 +1170,6 @@ value_assign (struct value *toval, struct value *fromval)
 	  }
 
 	observer_notify_register_changed (frame, value_reg);
-	if (deprecated_register_changed_hook)
-	  deprecated_register_changed_hook (-1);
 	break;
       }
 
-- 
2.5.0.rc0.5.g91e10c5.dirty

Re: [PATCH] tui: replace deprecated_register_changed_hook with observer

[Pedro Alves]

On 07/06/2015 02:17 AM, Patrick Palka wrote:
> This is a straightforward replacement of the TUI's use of the
> aforementioned hook with the register_changed observer.  Since this was
> the only user of the hook, this patch also removes the hook.
> 
> [ I am not sure if the changes to the function tui_register_changed are
>   correct.  In particular, the inputted frame argument is now passed down
>   to tui_check_data_values instead of the frame returned by
>   get_selected_frame.  The frame argument passed to each register_changed
>   observer corresponds to the VALUE_FRAME_ID of the register being
>   modified within a register assignment, e.g. the $rax in "print $rax =
>   FOO".  When would the frame corresponding to the VALUE_FRAME_ID of a
>   register not be the currently selected frame?  ]
> 

Grepping for value_assign callers finds e.g., varobjs:

  varobj.c:      val = value_assign (var->value, value);

Adding an assertion like this:

@@ -1169,6 +1169,7 @@ value_assign (struct value *toval, struct value *fromval)
              }
          }

+       gdb_assert (frame == get_selected_frame (NULL));
        observer_notify_register_changed (frame, value_reg);
        if (deprecated_register_changed_hook)
          deprecated_register_changed_hook (-1);

and playing with varobjs shows the assertion failing:

 (gdb) interpreter-exec mi "-var-create - * $rax"
 ^done,name="var1",numchild="0",value="6295640",type="int64_t",has_more="0"
 (gdb) up
 #1  0x000000000040082a in thread_function0 (arg=0x0) at threads.c:69
 69              usleep (1);  /* Loop increment.  */
 (gdb) up
 #2  0x0000003616a07ee5 in start_thread (arg=0x7ffff7fc1700) at pthread_create.c:309
 309           THREAD_SETMEM (pd, result, CALL_THREAD_FCT (pd));
 (gdb) interpreter-exec mi "-var-assign var1 1"
 ~"/home/pedro/gdb/mygit/build/../src/gdb/valops.c:1172: internal-error: value_assign: Assertion `frame == get_selected_frame (NULL)' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) "

The TUI doesn't use MI, but there are probably other similar cases
in the tree.  E.g., I'd assume you can create a register Value with Python,
and then assign to it when the selected frame is not
the register's frame.

Thanks,
Pedro Alves

Re: [PATCH] tui: replace deprecated_register_changed_hook with observer

[Patrick Palka]

On Wed, Jul 8, 2015 at 7:41 AM, Pedro Alves <palves@redhat.com> wrote:
> On 07/06/2015 02:17 AM, Patrick Palka wrote:
>> This is a straightforward replacement of the TUI's use of the
>> aforementioned hook with the register_changed observer.  Since this was
>> the only user of the hook, this patch also removes the hook.
>>
>> [ I am not sure if the changes to the function tui_register_changed are
>>   correct.  In particular, the inputted frame argument is now passed down
>>   to tui_check_data_values instead of the frame returned by
>>   get_selected_frame.  The frame argument passed to each register_changed
>>   observer corresponds to the VALUE_FRAME_ID of the register being
>>   modified within a register assignment, e.g. the $rax in "print $rax =
>>   FOO".  When would the frame corresponding to the VALUE_FRAME_ID of a
>>   register not be the currently selected frame?  ]
>>
>
> Grepping for value_assign callers finds e.g., varobjs:
>
>   varobj.c:      val = value_assign (var->value, value);
>
> Adding an assertion like this:
>
> @@ -1169,6 +1169,7 @@ value_assign (struct value *toval, struct value *fromval)
>               }
>           }
>
> +       gdb_assert (frame == get_selected_frame (NULL));
>         observer_notify_register_changed (frame, value_reg);
>         if (deprecated_register_changed_hook)
>           deprecated_register_changed_hook (-1);
>
> and playing with varobjs shows the assertion failing:
>
>  (gdb) interpreter-exec mi "-var-create - * $rax"
>  ^done,name="var1",numchild="0",value="6295640",type="int64_t",has_more="0"
>  (gdb) up
>  #1  0x000000000040082a in thread_function0 (arg=0x0) at threads.c:69
>  69              usleep (1);  /* Loop increment.  */
>  (gdb) up
>  #2  0x0000003616a07ee5 in start_thread (arg=0x7ffff7fc1700) at pthread_create.c:309
>  309           THREAD_SETMEM (pd, result, CALL_THREAD_FCT (pd));
>  (gdb) interpreter-exec mi "-var-assign var1 1"
>  ~"/home/pedro/gdb/mygit/build/../src/gdb/valops.c:1172: internal-error: value_assign: Assertion `frame == get_selected_frame (NULL)' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) "
>
> The TUI doesn't use MI, but there are probably other similar cases
> in the tree.  E.g., I'd assume you can create a register Value with Python,
> and then assign to it when the selected frame is not
> the register's frame.

Ah okay.. So it seems to me that if the frame argument !=
get_selected_frame, then we should not update the register window at
all since the register window is supposed to show the register values
of the currently selected frame.

Or instead, just ignore the frame argument and always pass
get_selected_frame to tui_check_data_values, even if frame !=
get_selected_frame.  Seems to me that this is the safest option.

Re: [PATCH] tui: replace deprecated_register_changed_hook with observer

[Pedro Alves]

On 07/08/2015 01:30 PM, Patrick Palka wrote:
> On Wed, Jul 8, 2015 at 7:41 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 07/06/2015 02:17 AM, Patrick Palka wrote:
>>> This is a straightforward replacement of the TUI's use of the
>>> aforementioned hook with the register_changed observer.  Since this was
>>> the only user of the hook, this patch also removes the hook.
>>>
>>> [ I am not sure if the changes to the function tui_register_changed are
>>>   correct.  In particular, the inputted frame argument is now passed down
>>>   to tui_check_data_values instead of the frame returned by
>>>   get_selected_frame.  The frame argument passed to each register_changed
>>>   observer corresponds to the VALUE_FRAME_ID of the register being
>>>   modified within a register assignment, e.g. the $rax in "print $rax =
>>>   FOO".  When would the frame corresponding to the VALUE_FRAME_ID of a
>>>   register not be the currently selected frame?  ]
>>>
>>
>> Grepping for value_assign callers finds e.g., varobjs:
>>
>>   varobj.c:      val = value_assign (var->value, value);
>>
>> Adding an assertion like this:
>>
>> @@ -1169,6 +1169,7 @@ value_assign (struct value *toval, struct value *fromval)
>>               }
>>           }
>>
>> +       gdb_assert (frame == get_selected_frame (NULL));
>>         observer_notify_register_changed (frame, value_reg);
>>         if (deprecated_register_changed_hook)
>>           deprecated_register_changed_hook (-1);
>>
>> and playing with varobjs shows the assertion failing:
>>
>>  (gdb) interpreter-exec mi "-var-create - * $rax"
>>  ^done,name="var1",numchild="0",value="6295640",type="int64_t",has_more="0"
>>  (gdb) up
>>  #1  0x000000000040082a in thread_function0 (arg=0x0) at threads.c:69
>>  69              usleep (1);  /* Loop increment.  */
>>  (gdb) up
>>  #2  0x0000003616a07ee5 in start_thread (arg=0x7ffff7fc1700) at pthread_create.c:309
>>  309           THREAD_SETMEM (pd, result, CALL_THREAD_FCT (pd));
>>  (gdb) interpreter-exec mi "-var-assign var1 1"
>>  ~"/home/pedro/gdb/mygit/build/../src/gdb/valops.c:1172: internal-error: value_assign: Assertion `frame == get_selected_frame (NULL)' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) "
>>
>> The TUI doesn't use MI, but there are probably other similar cases
>> in the tree.  E.g., I'd assume you can create a register Value with Python,
>> and then assign to it when the selected frame is not
>> the register's frame.
> 
> Ah okay.. So it seems to me that if the frame argument !=
> get_selected_frame, then we should not update the register window at
> all since the register window is supposed to show the register values
> of the currently selected frame.

Yes, I think so.

> Or instead, just ignore the frame argument and always pass
> get_selected_frame to tui_check_data_values, even if frame !=
> get_selected_frame.  Seems to me that this is the safest option.

That'd be a 1-1 with the current code.  Though, I believe
that results in spuriously clearing the highlight of
previously changed registers (of the selected frame), because
nothing will have changed.  So seems like the other option
actually fixes a bug.

Thanks,
Pedro Alves

Re: [PATCH] tui: replace deprecated_register_changed_hook with observer

[Patrick Palka]

On Wed, Jul 8, 2015 at 8:48 AM, Pedro Alves <palves@redhat.com> wrote:
> On 07/08/2015 01:30 PM, Patrick Palka wrote:
>> On Wed, Jul 8, 2015 at 7:41 AM, Pedro Alves <palves@redhat.com> wrote:
>>> On 07/06/2015 02:17 AM, Patrick Palka wrote:
>>>> This is a straightforward replacement of the TUI's use of the
>>>> aforementioned hook with the register_changed observer.  Since this was
>>>> the only user of the hook, this patch also removes the hook.
>>>>
>>>> [ I am not sure if the changes to the function tui_register_changed are
>>>>   correct.  In particular, the inputted frame argument is now passed down
>>>>   to tui_check_data_values instead of the frame returned by
>>>>   get_selected_frame.  The frame argument passed to each register_changed
>>>>   observer corresponds to the VALUE_FRAME_ID of the register being
>>>>   modified within a register assignment, e.g. the $rax in "print $rax =
>>>>   FOO".  When would the frame corresponding to the VALUE_FRAME_ID of a
>>>>   register not be the currently selected frame?  ]
>>>>
>>>
>>> Grepping for value_assign callers finds e.g., varobjs:
>>>
>>>   varobj.c:      val = value_assign (var->value, value);
>>>
>>> Adding an assertion like this:
>>>
>>> @@ -1169,6 +1169,7 @@ value_assign (struct value *toval, struct value *fromval)
>>>               }
>>>           }
>>>
>>> +       gdb_assert (frame == get_selected_frame (NULL));
>>>         observer_notify_register_changed (frame, value_reg);
>>>         if (deprecated_register_changed_hook)
>>>           deprecated_register_changed_hook (-1);
>>>
>>> and playing with varobjs shows the assertion failing:
>>>
>>>  (gdb) interpreter-exec mi "-var-create - * $rax"
>>>  ^done,name="var1",numchild="0",value="6295640",type="int64_t",has_more="0"
>>>  (gdb) up
>>>  #1  0x000000000040082a in thread_function0 (arg=0x0) at threads.c:69
>>>  69              usleep (1);  /* Loop increment.  */
>>>  (gdb) up
>>>  #2  0x0000003616a07ee5 in start_thread (arg=0x7ffff7fc1700) at pthread_create.c:309
>>>  309           THREAD_SETMEM (pd, result, CALL_THREAD_FCT (pd));
>>>  (gdb) interpreter-exec mi "-var-assign var1 1"
>>>  ~"/home/pedro/gdb/mygit/build/../src/gdb/valops.c:1172: internal-error: value_assign: Assertion `frame == get_selected_frame (NULL)' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) "
>>>
>>> The TUI doesn't use MI, but there are probably other similar cases
>>> in the tree.  E.g., I'd assume you can create a register Value with Python,
>>> and then assign to it when the selected frame is not
>>> the register's frame.
>>
>> Ah okay.. So it seems to me that if the frame argument !=
>> get_selected_frame, then we should not update the register window at
>> all since the register window is supposed to show the register values
>> of the currently selected frame.
>
> Yes, I think so.
>
>> Or instead, just ignore the frame argument and always pass
>> get_selected_frame to tui_check_data_values, even if frame !=
>> get_selected_frame.  Seems to me that this is the safest option.
>
> That'd be a 1-1 with the current code.  Though, I believe
> that results in spuriously clearing the highlight of
> previously changed registers (of the selected frame), because
> nothing will have changed.  So seems like the other option
> actually fixes a bug.

Is it actually the case that a register change made on one frame can
not show up on some other frame?

If I debug gdb with gdb, doing "start" followed by "step" a couple
dozen times, do "layout regs", then select the outermost frame and do
"print $rbx = 50", the regs window shows that $rbx has not changed on
the (selected) outermost frame but if i select the innermost frame,
$rbx has changed to 50.  And the frame_id of the register $rbx was
indeed the (selected) outermost frame, yet the registers of the
selected frame did not change after the value assignment and the
registers of some other frame did.  I don't know why this particular
example behaves this way, but it seems to illustrates that it's
possible that a register change made in one frame can affect the
register values of another frame.  So I don't know if the "frame !=
get_selected_frame ()" check is 100% correct.

Re: [PATCH] tui: replace deprecated_register_changed_hook with observer

[Pedro Alves]

On 07/08/2015 02:37 PM, Patrick Palka wrote:
> On Wed, Jul 8, 2015 at 8:48 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 07/08/2015 01:30 PM, Patrick Palka wrote:
>>> On Wed, Jul 8, 2015 at 7:41 AM, Pedro Alves <palves@redhat.com> wrote:
>>>> On 07/06/2015 02:17 AM, Patrick Palka wrote:
>>>>> This is a straightforward replacement of the TUI's use of the
>>>>> aforementioned hook with the register_changed observer.  Since this was
>>>>> the only user of the hook, this patch also removes the hook.
>>>>>
>>>>> [ I am not sure if the changes to the function tui_register_changed are
>>>>>   correct.  In particular, the inputted frame argument is now passed down
>>>>>   to tui_check_data_values instead of the frame returned by
>>>>>   get_selected_frame.  The frame argument passed to each register_changed
>>>>>   observer corresponds to the VALUE_FRAME_ID of the register being
>>>>>   modified within a register assignment, e.g. the $rax in "print $rax =
>>>>>   FOO".  When would the frame corresponding to the VALUE_FRAME_ID of a
>>>>>   register not be the currently selected frame?  ]
>>>>>
>>>>
>>>> Grepping for value_assign callers finds e.g., varobjs:
>>>>
>>>>   varobj.c:      val = value_assign (var->value, value);
>>>>
>>>> Adding an assertion like this:
>>>>
>>>> @@ -1169,6 +1169,7 @@ value_assign (struct value *toval, struct value *fromval)
>>>>               }
>>>>           }
>>>>
>>>> +       gdb_assert (frame == get_selected_frame (NULL));
>>>>         observer_notify_register_changed (frame, value_reg);
>>>>         if (deprecated_register_changed_hook)
>>>>           deprecated_register_changed_hook (-1);
>>>>
>>>> and playing with varobjs shows the assertion failing:
>>>>
>>>>  (gdb) interpreter-exec mi "-var-create - * $rax"
>>>>  ^done,name="var1",numchild="0",value="6295640",type="int64_t",has_more="0"
>>>>  (gdb) up
>>>>  #1  0x000000000040082a in thread_function0 (arg=0x0) at threads.c:69
>>>>  69              usleep (1);  /* Loop increment.  */
>>>>  (gdb) up
>>>>  #2  0x0000003616a07ee5 in start_thread (arg=0x7ffff7fc1700) at pthread_create.c:309
>>>>  309           THREAD_SETMEM (pd, result, CALL_THREAD_FCT (pd));
>>>>  (gdb) interpreter-exec mi "-var-assign var1 1"
>>>>  ~"/home/pedro/gdb/mygit/build/../src/gdb/valops.c:1172: internal-error: value_assign: Assertion `frame == get_selected_frame (NULL)' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) "
>>>>
>>>> The TUI doesn't use MI, but there are probably other similar cases
>>>> in the tree.  E.g., I'd assume you can create a register Value with Python,
>>>> and then assign to it when the selected frame is not
>>>> the register's frame.
>>>
>>> Ah okay.. So it seems to me that if the frame argument !=
>>> get_selected_frame, then we should not update the register window at
>>> all since the register window is supposed to show the register values
>>> of the currently selected frame.
>>
>> Yes, I think so.
>>
>>> Or instead, just ignore the frame argument and always pass
>>> get_selected_frame to tui_check_data_values, even if frame !=
>>> get_selected_frame.  Seems to me that this is the safest option.
>>
>> That'd be a 1-1 with the current code.  Though, I believe
>> that results in spuriously clearing the highlight of
>> previously changed registers (of the selected frame), because
>> nothing will have changed.  So seems like the other option
>> actually fixes a bug.
> 
> Is it actually the case that a register change made on one frame can
> not show up on some other frame?

Oh, you're right, good point.   Registers can well be at the
same physical location across frames.

> 
> If I debug gdb with gdb, doing "start" followed by "step" a couple
> dozen times, do "layout regs", then select the outermost frame and do
> "print $rbx = 50", the regs window shows that $rbx has not changed on
> the (selected) outermost frame but if i select the innermost frame,
> $rbx has changed to 50.  And the frame_id of the register $rbx was
> indeed the (selected) outermost frame, yet the registers of the
> selected frame did not change after the value assignment and the
> registers of some other frame did.  I don't know why this particular
> example behaves this way, but it seems to illustrates that it's
> possible that a register change made in one frame can affect the
> register values of another frame.  So I don't know if the "frame !=
> get_selected_frame ()" check is 100% correct.
> 

Yeah.  Sorry, somehow forgot this...

Thanks,
Pedro Alves

[PATCH] tui: replace deprecated_register_changed_hook with observer

[Patrick Palka]

This is a straightforward replacement of the TUI's use of the
aforementioned hook with the register_changed observer.  Since this was
the only user of the hook, this patch also removes the hook.

gdb/ChangeLog:

	* defs.h (deprecated_register_changed_hook): Remove prototype.
	* interps.c (clear_iterpreter_hooks): Remove reference to
	deprecated_register_changed_hook.
	* top.c (deprecated_register_changed_hook): Remove prototype.
	* valops.c (value_assign): Remove reference to
	deprecated_register_changed_hook.
	* tui/tui-hooks.c (tui_register_changed): Add parameter "frame".
	Add comment documenting the function.
	(tui_register_changed_observer): Define.
	(tui_install_hooks): Remove reference to
	deprecated_register_changed_hook.  Set
	tui_register_changed_observer.
	(tui_remove_hooks): Remove reference to
	deprecated_register_changed_hook.  Unset
	tui_register_changed_observer.
---
 gdb/defs.h          |  1 -
 gdb/interps.c       |  1 -
 gdb/top.c           |  5 -----
 gdb/tui/tui-hooks.c | 18 +++++++++++++-----
 gdb/valops.c        |  2 --
 5 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/gdb/defs.h b/gdb/defs.h
index 32b08bb..a555da1 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -649,7 +649,6 @@ extern void (*deprecated_readline_begin_hook) (char *, ...)
      ATTRIBUTE_FPTR_PRINTF_1;
 extern char *(*deprecated_readline_hook) (const char *);
 extern void (*deprecated_readline_end_hook) (void);
-extern void (*deprecated_register_changed_hook) (int regno);
 extern void (*deprecated_context_hook) (int);
 extern ptid_t (*deprecated_target_wait_hook) (ptid_t ptid,
 					      struct target_waitstatus *status,
diff --git a/gdb/interps.c b/gdb/interps.c
index 4c1e6cc..d825e14 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -370,7 +370,6 @@ clear_interpreter_hooks (void)
   deprecated_readline_begin_hook = 0;
   deprecated_readline_hook = 0;
   deprecated_readline_end_hook = 0;
-  deprecated_register_changed_hook = 0;
   deprecated_context_hook = 0;
   deprecated_target_wait_hook = 0;
   deprecated_call_command_hook = 0;
diff --git a/gdb/top.c b/gdb/top.c
index 01fddd2..1e30b1c 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -221,11 +221,6 @@ void (*deprecated_detach_hook) (void);
 
 void (*deprecated_interactive_hook) (void);
 
-/* Tell the GUI someone changed the register REGNO.  -1 means
-   that the caller does not know which register changed or
-   that several registers have changed (see value_assign).  */
-void (*deprecated_register_changed_hook) (int regno);
-
 /* Called when going to wait for the target.  Usually allows the GUI
    to run while waiting for target events.  */
 
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 0eb2f07..c885108 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -66,11 +66,18 @@ tui_new_objfile_hook (struct objfile* objfile)
 /* Prevent recursion of deprecated_register_changed_hook().  */
 static int tui_refreshing_registers = 0;
 
+/* Observer for the register_changed notification.  */
+
 static void
-tui_register_changed_hook (int regno)
+tui_register_changed (struct frame_info *frame, int regno)
 {
   struct frame_info *fi;
 
+  /* The frame of the register that was changed may differ from the selected
+     frame, but we only want to show the register values of the selected frame.
+     And even if the frames differ a register change made in one can still show
+     up in the other.  So we always use the selected frame here, and ignore
+     FRAME.  */
   fi = get_selected_frame (NULL);
   if (tui_refreshing_registers == 0)
     {
@@ -226,6 +233,7 @@ static struct observer *tui_inferior_exit_observer;
 static struct observer *tui_about_to_proceed_observer;
 static struct observer *tui_before_prompt_observer;
 static struct observer *tui_normal_stop_observer;
+static struct observer *tui_register_changed_observer;
 
 /* Install the TUI specific hooks.  */
 void
@@ -253,8 +261,8 @@ tui_install_hooks (void)
     = observer_attach_before_prompt (tui_before_prompt);
   tui_normal_stop_observer
     = observer_attach_normal_stop (tui_normal_stop);
-
-  deprecated_register_changed_hook = tui_register_changed_hook;
+  tui_register_changed_observer
+    = observer_attach_register_changed (tui_register_changed);
 }
 
 /* Remove the TUI specific hooks.  */
@@ -263,8 +271,6 @@ tui_remove_hooks (void)
 {
   deprecated_print_frame_info_listing_hook = 0;
   deprecated_query_hook = 0;
-  deprecated_register_changed_hook = 0;
-
   /* Remove our observers.  */
   observer_detach_breakpoint_created (tui_bp_created_observer);
   tui_bp_created_observer = NULL;
@@ -280,6 +286,8 @@ tui_remove_hooks (void)
   tui_before_prompt_observer = NULL;
   observer_detach_normal_stop (tui_normal_stop_observer);
   tui_normal_stop_observer = NULL;
+  observer_detach_register_changed (tui_register_changed_observer);
+  tui_register_changed_observer = NULL;
 }
 
 void _initialize_tui_hooks (void);
diff --git a/gdb/valops.c b/gdb/valops.c
index 50082c9..403e088 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1170,8 +1170,6 @@ value_assign (struct value *toval, struct value *fromval)
 	  }
 
 	observer_notify_register_changed (frame, value_reg);
-	if (deprecated_register_changed_hook)
-	  deprecated_register_changed_hook (-1);
 	break;
       }
 
-- 
2.5.0.rc0.5.g91e10c5.dirty

Re: [PATCH] tui: replace deprecated_register_changed_hook with observer

[Pedro Alves]

On 07/08/2015 03:11 PM, Patrick Palka wrote:
> This is a straightforward replacement of the TUI's use of the
> aforementioned hook with the register_changed observer.  Since this was
> the only user of the hook, this patch also removes the hook.

OK.

(Note: I'm aware that insight uses some of the these deprecated
hooks, but as we're showing how to convert them to observers at
the same time, I think that's fine; insight can be converted
as it pulls in newer gdb; otherwise it seems like at the pace
it's been taking, we'd never manage to get rid of the
deprecated hooks.)

Thanks,
Pedro Alves