diff mbox series

[07/14] Simplify update_register_data

Message ID 20231217-tui-regs-cleanup-v1-7-67bd0ea1e8be@tromey.com
State New
Headers
Series Cleanups for the TUi register window |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Tom Tromey Dec. 17, 2023, 7:50 p.m. UTC 
  This changes update_register_data to always update the register data.
The idea here is that this is really only called when either the
desired register group changes, or when gdb transitions from not
having a frame to having a frame.

show_registers is also simplified slightly to account for this.
---
 gdb/tui/tui-regs.c | 57 ++++++++++++++++++++----------------------------------
 1 file changed, 21 insertions(+), 36 deletions(-)

Comments

Andrew Burgess Dec. 18, 2023, 7:48 p.m. UTC | #1 
Tom Tromey <tom@tromey.com> writes:

> This changes update_register_data to always update the register data.
> The idea here is that this is really only called when either the
> desired register group changes, or when gdb transitions from not
> having a frame to having a frame.
>
> show_registers is also simplified slightly to account for this.
> ---
>  gdb/tui/tui-regs.c | 57 ++++++++++++++++++++----------------------------------
>  1 file changed, 21 insertions(+), 36 deletions(-)
>
> diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
> index 0ad23e93778..7b6b669fe51 100644
> --- a/gdb/tui/tui-regs.c
> +++ b/gdb/tui/tui-regs.c
> @@ -170,15 +170,10 @@ tui_data_window::show_registers (const reggroup *group)
>      group = general_reggroup;
>  
>    if (target_has_registers () && target_has_stack () && target_has_memory ())
> -    {
> -      update_register_data (group, get_selected_frame (NULL));
> -
> -      /* Clear all notation of changed values.  */
> -      for (auto &&data_item_win : m_regs_content)
> -	data_item_win.highlight = false;
> -    }
> +    update_register_data (group, get_selected_frame (nullptr));
>    else
>      {
> +      set_title (_("Registers"));

It's strange that you add this line, but in the very next commit change
the text to the empty string.  Maybe I'm missing something, but it feels
like you could just set the empty string here maybe?

Thanks,
Andrew


>        m_current_group = nullptr;
>        m_regs_content.clear ();
>      }
> @@ -195,40 +190,30 @@ void
>  tui_data_window::update_register_data (const reggroup *group,
>  				       frame_info_ptr frame)
>  {
> -  if (group != m_current_group)
> -    {
> -      m_current_group = group;
> +  m_current_group = group;
>  
> -      /* Make a new title showing which group we display.  */
> -      this->set_title (string_printf ("Register group: %s", group->name ()));
> +  /* Make a new title showing which group we display.  */
> +  this->set_title (string_printf ("Register group: %s", group->name ()));
>  
> -      /* Create the registers.  */
> -      m_regs_content.clear ();
> +  /* Create the registers.  */
> +  m_regs_content.clear ();
>  
> -      struct gdbarch *gdbarch = get_frame_arch (frame);
> -      for (int regnum = 0;
> -	   regnum < gdbarch_num_cooked_regs (gdbarch);
> -	   regnum++)
> -	{
> -	  /* Must be in the group.  */
> -	  if (!gdbarch_register_reggroup_p (gdbarch, regnum, group))
> -	    continue;
> +  struct gdbarch *gdbarch = get_frame_arch (frame);
> +  for (int regnum = 0;
> +       regnum < gdbarch_num_cooked_regs (gdbarch);
> +       regnum++)
> +    {
> +      /* Must be in the group.  */
> +      if (!gdbarch_register_reggroup_p (gdbarch, regnum, group))
> +	continue;
>  
> -	  /* If the register name is empty, it is undefined for this
> -	     processor, so don't display anything.  */
> -	  const char *name = gdbarch_register_name (gdbarch, regnum);
> -	  if (*name == '\0')
> -	    continue;
> +      /* If the register name is empty, it is undefined for this
> +	 processor, so don't display anything.  */
> +      const char *name = gdbarch_register_name (gdbarch, regnum);
> +      if (*name == '\0')
> +	continue;
>  
> -	  m_regs_content.emplace_back (regnum, frame);
> -	}
> -    }
> -  else
> -    {
> -      /* The group did not change, so we can simply update each
> -	 item. */
> -      for (tui_register_info &reg : m_regs_content)
> -	reg.update (frame);
> +      m_regs_content.emplace_back (regnum, frame);
>      }
>  }
>  
>
> -- 
> 2.43.0
Tom Tromey Jan. 20, 2024, 5:06 p.m. UTC | #2 
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

>> +      set_title (_("Registers"));

Andrew> It's strange that you add this line, but in the very next commit change
Andrew> the text to the empty string.  Maybe I'm missing something, but it feels
Andrew> like you could just set the empty string here maybe?

I think I meant to fix this up and forgot.  Anyway I've done it now.

Tom
diff mbox series

Patch

diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 0ad23e93778..7b6b669fe51 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -170,15 +170,10 @@  tui_data_window::show_registers (const reggroup *group)
     group = general_reggroup;
 
   if (target_has_registers () && target_has_stack () && target_has_memory ())
-    {
-      update_register_data (group, get_selected_frame (NULL));
-
-      /* Clear all notation of changed values.  */
-      for (auto &&data_item_win : m_regs_content)
-	data_item_win.highlight = false;
-    }
+    update_register_data (group, get_selected_frame (nullptr));
   else
     {
+      set_title (_("Registers"));
       m_current_group = nullptr;
       m_regs_content.clear ();
     }
@@ -195,40 +190,30 @@  void
 tui_data_window::update_register_data (const reggroup *group,
 				       frame_info_ptr frame)
 {
-  if (group != m_current_group)
-    {
-      m_current_group = group;
+  m_current_group = group;
 
-      /* Make a new title showing which group we display.  */
-      this->set_title (string_printf ("Register group: %s", group->name ()));
+  /* Make a new title showing which group we display.  */
+  this->set_title (string_printf ("Register group: %s", group->name ()));
 
-      /* Create the registers.  */
-      m_regs_content.clear ();
+  /* Create the registers.  */
+  m_regs_content.clear ();
 
-      struct gdbarch *gdbarch = get_frame_arch (frame);
-      for (int regnum = 0;
-	   regnum < gdbarch_num_cooked_regs (gdbarch);
-	   regnum++)
-	{
-	  /* Must be in the group.  */
-	  if (!gdbarch_register_reggroup_p (gdbarch, regnum, group))
-	    continue;
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+  for (int regnum = 0;
+       regnum < gdbarch_num_cooked_regs (gdbarch);
+       regnum++)
+    {
+      /* Must be in the group.  */
+      if (!gdbarch_register_reggroup_p (gdbarch, regnum, group))
+	continue;
 
-	  /* If the register name is empty, it is undefined for this
-	     processor, so don't display anything.  */
-	  const char *name = gdbarch_register_name (gdbarch, regnum);
-	  if (*name == '\0')
-	    continue;
+      /* If the register name is empty, it is undefined for this
+	 processor, so don't display anything.  */
+      const char *name = gdbarch_register_name (gdbarch, regnum);
+      if (*name == '\0')
+	continue;
 
-	  m_regs_content.emplace_back (regnum, frame);
-	}
-    }
-  else
-    {
-      /* The group did not change, so we can simply update each
-	 item. */
-      for (tui_register_info &reg : m_regs_content)
-	reg.update (frame);
+      m_regs_content.emplace_back (regnum, frame);
     }
 }