[PATCH 00/14] TUI refactoring round N

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 00/14] TUI refactoring round N

Tom Tromey-2
Here's yet another series of TUI refactorings.  This series should
have no user-visible changes.  There are several more series like this
to come, but as before, they are interspersed with user-visible
patches which I plan to submit separately.

Each patch was built and tested using gdb.tui on x86-64 Fedora 28.

Tom


Reply | Threaded
Open this post in threaded view
|

[PATCH 01/14] Some i18n fixes for the TUI

Tom Tromey-2
The TUI has a few #defines that hold user-visible strings.  As these
are only used in a single spot, this patch removes the defines,
preferring direct use of the string where needed.  Furthermore, now
the strings are wrapped in _(), which is friendlier for i18n purposes.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-source.h (struct tui_source_window): Update.
        * tui/tui-regs.c (tui_show_registers): Update.
        * tui/tui-disasm.h (struct tui_disasm_window): Update.
        * tui/tui-data.h (NO_SRC_STRING, NO_DISASSEM_STRING)
        (NO_REGS_STRING): Remove defines.
---
 gdb/ChangeLog        | 8 ++++++++
 gdb/tui/tui-data.h   | 3 ---
 gdb/tui/tui-disasm.h | 2 +-
 gdb/tui/tui-regs.c   | 2 +-
 gdb/tui/tui-source.h | 2 +-
 5 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 7993c639376..0432a53750f 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -103,9 +103,6 @@ public:
 
 /* Constant definitions.  */
 #define DEFAULT_TAB_LEN         8
-#define NO_SRC_STRING           "[ No Source Available ]"
-#define NO_DISASSEM_STRING      "[ No Assembly Available ]"
-#define NO_REGS_STRING          "[ Register Values Unavailable ]"
 #define NO_DATA_STRING          "[ No Data Values Displayed ]"
 #define SRC_NAME                "src"
 #define CMD_NAME                "cmd"
diff --git a/gdb/tui/tui-disasm.h b/gdb/tui/tui-disasm.h
index d9895322487..bfddfa01837 100644
--- a/gdb/tui/tui-disasm.h
+++ b/gdb/tui/tui-disasm.h
@@ -50,7 +50,7 @@ struct tui_disasm_window : public tui_source_window_base
 
   void erase_source_content () override
   {
-    do_erase_source_content (NO_DISASSEM_STRING);
+    do_erase_source_content (_("[ No Assembly Available ]"));
   }
 
 protected:
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 36973ff51ae..8fcb7bc46bc 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -165,7 +165,7 @@ tui_show_registers (struct reggroup *group)
   else
     {
       TUI_DATA_WIN->current_group = 0;
-      TUI_DATA_WIN->erase_data_content (NO_REGS_STRING);
+      TUI_DATA_WIN->erase_data_content (_("[ Register Values Unavailable ]"));
     }
 }
 
diff --git a/gdb/tui/tui-source.h b/gdb/tui/tui-source.h
index 9c3013637b7..a7002123c97 100644
--- a/gdb/tui/tui-source.h
+++ b/gdb/tui/tui-source.h
@@ -53,7 +53,7 @@ struct tui_source_window : public tui_source_window_base
 
   void erase_source_content () override
   {
-    do_erase_source_content (NO_SRC_STRING);
+    do_erase_source_content (_("[ No Source Available ]"));
   }
 
   void show_symtab_source (struct gdbarch *, struct symtab *,
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 02/14] Remove NULL check from tui_reg_command

Tom Tromey-2
In reply to this post by Tom Tromey-2
tui_reg_command has an unnecessary NULL check.  The preceding call to
tui_reg_layout will ensure the window exists.  This patch removes the
check.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-regs.c (tui_reg_command): Remove NULL check.
---
 gdb/ChangeLog      | 4 ++++
 gdb/tui/tui-regs.c | 4 +---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 8fcb7bc46bc..b3c7ce627b4 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -669,9 +669,7 @@ tui_reg_command (const char *args, int from_tty)
       if (TUI_DATA_WIN == NULL || !TUI_DATA_WIN->is_visible ())
  tui_reg_layout ();
 
-      struct reggroup *current_group = NULL;
-      if (TUI_DATA_WIN != NULL)
- current_group = TUI_DATA_WIN->current_group;
+      struct reggroup *current_group = TUI_DATA_WIN->current_group;
       if (strncmp (args, "next", len) == 0)
  match = tui_reg_next (current_group, gdbarch);
       else if (strncmp (args, "prev", len) == 0)
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 03/14] Minor rearrangement in tui-regs.c

Tom Tromey-2
In reply to this post by Tom Tromey-2
This moves a couple of functions earlier in tui-regs.c.  Previously
they were in the "command" section of the file, but really they belong
in the "window implementation" section.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-regs.c (tui_register_format, tui_get_register): Move
        earlier.
---
 gdb/ChangeLog      |   5 +++
 gdb/tui/tui-regs.c | 101 +++++++++++++++++++++------------------------
 2 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index b3c7ce627b4..9ea6e723644 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -49,10 +49,55 @@ static void tui_show_register_group (tui_data_window *win_info,
      struct frame_info *frame,
      int refresh_values_only);
 
-static void tui_get_register (struct frame_info *frame,
-      struct tui_data_item_window *data,
-      int regnum, bool *changedp);
+/* Get the register from the frame and return a printable
+   representation of it.  */
+
+static char *
+tui_register_format (struct frame_info *frame, int regnum)
+{
+  struct gdbarch *gdbarch = get_frame_arch (frame);
 
+  string_file stream;
+
+  scoped_restore save_pagination
+    = make_scoped_restore (&pagination_enabled, 0);
+  scoped_restore save_stdout
+    = make_scoped_restore (&gdb_stdout, &stream);
+
+  gdbarch_print_registers_info (gdbarch, &stream, frame, regnum, 1);
+
+  /* Remove the possible \n.  */
+  std::string &str = stream.string ();
+  if (!str.empty () && str.back () == '\n')
+    str.resize (str.size () - 1);
+
+  /* Expand tabs into spaces, since ncurses on MS-Windows doesn't.  */
+  return tui_expand_tabs (str.c_str (), 0);
+}
+
+/* Get the register value from the given frame and format it for the
+   display.  When changep is set, check if the new register value has
+   changed with respect to the previous call.  */
+static void
+tui_get_register (struct frame_info *frame,
+                  struct tui_data_item_window *data,
+  int regnum, bool *changedp)
+{
+  if (changedp)
+    *changedp = false;
+  if (target_has_registers)
+    {
+      char *prev_content = data->content;
+
+      data->content = tui_register_format (frame, regnum);
+
+      if (changedp != NULL
+  && strcmp (prev_content, data->content) != 0)
+ *changedp = true;
+
+      xfree (prev_content);
+    }
+}
 
 /* See tui-regs.h.  */
 
@@ -739,56 +784,6 @@ tui_reggroup_completer (struct cmd_list_element *ignore,
     }
 }
 
-/* Get the register from the frame and return a printable
-   representation of it.  */
-
-static char *
-tui_register_format (struct frame_info *frame, int regnum)
-{
-  struct gdbarch *gdbarch = get_frame_arch (frame);
-
-  string_file stream;
-
-  scoped_restore save_pagination
-    = make_scoped_restore (&pagination_enabled, 0);
-  scoped_restore save_stdout
-    = make_scoped_restore (&gdb_stdout, &stream);
-
-  gdbarch_print_registers_info (gdbarch, &stream, frame, regnum, 1);
-
-  /* Remove the possible \n.  */
-  std::string &str = stream.string ();
-  if (!str.empty () && str.back () == '\n')
-    str.resize (str.size () - 1);
-
-  /* Expand tabs into spaces, since ncurses on MS-Windows doesn't.  */
-  return tui_expand_tabs (str.c_str (), 0);
-}
-
-/* Get the register value from the given frame and format it for the
-   display.  When changep is set, check if the new register value has
-   changed with respect to the previous call.  */
-static void
-tui_get_register (struct frame_info *frame,
-                  struct tui_data_item_window *data,
-  int regnum, bool *changedp)
-{
-  if (changedp)
-    *changedp = false;
-  if (target_has_registers)
-    {
-      char *prev_content = data->content;
-
-      data->content = tui_register_format (frame, regnum);
-
-      if (changedp != NULL
-  && strcmp (prev_content, data->content) != 0)
- *changedp = true;
-
-      xfree (prev_content);
-    }
-}
-
 void
 _initialize_tui_regs (void)
 {
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 04/14] Remove tui_data_item_window::value

Tom Tromey-2
In reply to this post by Tom Tromey-2
The field tui_data_item_window::value is not used, so remove it.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-regs.h (struct tui_data_item_window) <value>: Remove
        field.
        * tui/tui-regs.c (~tui_data_item_window): Update.
---
 gdb/ChangeLog      | 6 ++++++
 gdb/tui/tui-regs.c | 1 -
 gdb/tui/tui-regs.h | 1 -
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 9ea6e723644..aebea49effa 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -103,7 +103,6 @@ tui_get_register (struct frame_info *frame,
 
 tui_data_item_window::~tui_data_item_window ()
 {
-  xfree (value);
   xfree (content);
 }
 
diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index 7b0bb505a8e..d54b556148e 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -38,7 +38,6 @@ struct tui_data_item_window : public tui_gen_win_info
   const char *name = nullptr;
   /* The register number, or data display number.  */
   int item_no = -1;
-  void *value = nullptr;
   bool highlight = false;
   char *content = nullptr;
 };
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 05/14] Change tui_data_item_window::content to be a unique_xmalloc_ptr

Tom Tromey-2
In reply to this post by Tom Tromey-2
This changes tui_data_item_window::content to be a unique_xmalloc_ptr
and fixes up the fallout.  It also removes a parameter from
tui_expand_tabs, as it was only ever given one value.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-regs.h (struct tui_data_item_window)
        <~tui_data_item_window>: Remove.
        <content>: Now a unique_xmalloc_ptr.
        * tui/tui-regs.c (tui_register_format): Return a
        unique_xmalloc_ptr.
        (tui_get_register): Update.
        (~tui_data_item_window): Remove.
        (tui_data_window::display_registers_from, tui_display_register):
        Update.
        * tui/tui-io.h (tui_expand_tabs): Update.
        * tui/tui-io.c (tui_expand_tabs): Return a unique_xmalloc_ptr.
        Remove "col" parameter.
---
 gdb/ChangeLog      | 15 +++++++++++++++
 gdb/tui/tui-io.c   | 14 ++++++--------
 gdb/tui/tui-io.h   |  2 +-
 gdb/tui/tui-regs.c | 34 ++++++++++------------------------
 gdb/tui/tui-regs.h |  4 +---
 5 files changed, 33 insertions(+), 36 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 7bdba3f64ff..ac7f0982755 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -1050,19 +1050,17 @@ tui_getc (FILE *fp)
   return ch;
 }
 
-/* Utility function to expand TABs in a STRING into spaces.  STRING
-   will be displayed starting at column COL, and is assumed to include
-   no newlines.  The returned expanded string is malloc'ed.  */
+/* See tui-io.h.  */
 
-char *
-tui_expand_tabs (const char *string, int col)
+gdb::unique_xmalloc_ptr<char>
+tui_expand_tabs (const char *string)
 {
   int n_adjust, ncol;
   const char *s;
   char *ret, *q;
 
   /* 1. How many additional characters do we need?  */
-  for (ncol = col, n_adjust = 0, s = string; s; )
+  for (ncol = 0, n_adjust = 0, s = string; s; )
     {
       s = strpbrk (s, "\t");
       if (s)
@@ -1079,7 +1077,7 @@ tui_expand_tabs (const char *string, int col)
   ret = q = (char *) xmalloc (strlen (string) + n_adjust + 1);
 
   /* 2. Copy the original string while replacing TABs with spaces.  */
-  for (ncol = col, s = string; s; )
+  for (ncol = 0, s = string; s; )
     {
       const char *s1 = strpbrk (s, "\t");
       if (s1)
@@ -1101,5 +1099,5 @@ tui_expand_tabs (const char *string, int col)
       s = s1;
     }
 
-  return ret;
+  return gdb::unique_xmalloc_ptr<char> (ret);
 }
diff --git a/gdb/tui/tui-io.h b/gdb/tui/tui-io.h
index 34a24da2923..ec2378759a2 100644
--- a/gdb/tui/tui-io.h
+++ b/gdb/tui/tui-io.h
@@ -46,7 +46,7 @@ extern void tui_initialize_io (void);
 extern void tui_redisplay_readline (void);
 
 /* Expand TABs into spaces.  */
-extern char *tui_expand_tabs (const char *, int);
+extern gdb::unique_xmalloc_ptr<char> tui_expand_tabs (const char *);
 
 /* Enter/leave reverse video mode.  */
 extern void tui_set_reverse_mode (WINDOW *w, bool reverse);
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index aebea49effa..a899b1df694 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -52,7 +52,7 @@ static void tui_show_register_group (tui_data_window *win_info,
 /* Get the register from the frame and return a printable
    representation of it.  */
 
-static char *
+static gdb::unique_xmalloc_ptr<char>
 tui_register_format (struct frame_info *frame, int regnum)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
@@ -72,7 +72,7 @@ tui_register_format (struct frame_info *frame, int regnum)
     str.resize (str.size () - 1);
 
   /* Expand tabs into spaces, since ncurses on MS-Windows doesn't.  */
-  return tui_expand_tabs (str.c_str (), 0);
+  return tui_expand_tabs (str.c_str ());
 }
 
 /* Get the register value from the given frame and format it for the
@@ -87,27 +87,19 @@ tui_get_register (struct frame_info *frame,
     *changedp = false;
   if (target_has_registers)
     {
-      char *prev_content = data->content;
-
-      data->content = tui_register_format (frame, regnum);
+      gdb::unique_xmalloc_ptr<char> new_content
+ = tui_register_format (frame, regnum);
 
       if (changedp != NULL
-  && strcmp (prev_content, data->content) != 0)
+  && strcmp (data->content.get (), new_content.get ()) != 0)
  *changedp = true;
 
-      xfree (prev_content);
+      data->content = std::move (new_content);
     }
 }
 
 /* See tui-regs.h.  */
 
-tui_data_item_window::~tui_data_item_window ()
-{
-  xfree (content);
-}
-
-/* See tui-regs.h.  */
-
 int
 tui_data_window::last_regs_line_no () const
 {
@@ -309,19 +301,13 @@ tui_data_window::display_registers_from (int start_element_no)
       int max_len = 0;
       for (auto &&data_item_win : regs_content)
         {
-          char *p;
+          const char *p;
           int len;
 
           len = 0;
-          p = data_item_win->content;
+          p = data_item_win->content.get ();
           if (p != 0)
-            while (*p)
-              {
-                if (*p++ == '\t')
-                  len = 8 * ((len / 8) + 1);
-                else
-                  len++;
-              }
+    len = strlen (p);
 
           if (len > max_len)
             max_len = len;
@@ -641,7 +627,7 @@ tui_display_register (struct tui_data_item_window *data)
         waddch (data->handle, ' ');
       wmove (data->handle, 0, 0);
       if (data->content)
-        waddstr (data->handle, data->content);
+        waddstr (data->handle, data->content.get ());
 
       if (data->highlight)
  /* We ignore the return value, casting it to void in order to avoid
diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index d54b556148e..b70d8df3620 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -33,13 +33,11 @@ struct tui_data_item_window : public tui_gen_win_info
   {
   }
 
-  ~tui_data_item_window () override;
-
   const char *name = nullptr;
   /* The register number, or data display number.  */
   int item_no = -1;
   bool highlight = false;
-  char *content = nullptr;
+  gdb::unique_xmalloc_ptr<char> content;
 };
 
 /* The TUI registers window.  */
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 06/14] Rearrange tui-regs.c some more

Tom Tromey-2
In reply to this post by Tom Tromey-2
This moves tui_reg_layout later in tui-regs.c, closer to where it is
used.

It also changes tui_show_registers not to enable the TUI or change the
layout -- this is already done by this point by all the callers.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-regs.c (tui_reg_layout): Move later.
        (tui_show_registers): Don't enable TUI mode or change layout.
---
 gdb/ChangeLog      |  5 +++++
 gdb/tui/tui-regs.c | 38 +++++++++++++++-----------------------
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index a899b1df694..147f57a13a2 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -149,34 +149,11 @@ tui_data_window::first_reg_element_no_inline (int line_no) const
     return (-1);
 }
 
-/* A helper function to display the register window in the appropriate
-   way.  */
-
-static void
-tui_reg_layout ()
-{
-  enum tui_layout_type cur_layout = tui_current_layout ();
-  enum tui_layout_type new_layout;
-  if (cur_layout == SRC_COMMAND || cur_layout == SRC_DATA_COMMAND)
-    new_layout = SRC_DATA_COMMAND;
-  else
-    new_layout = DISASSEM_DATA_COMMAND;
-  tui_set_layout (new_layout);
-}
-
 /* Show the registers of the given group in the data window
    and refresh the window.  */
 void
 tui_show_registers (struct reggroup *group)
 {
-  /* Make sure the curses mode is enabled.  */
-  tui_enable ();
-
-  /* Make sure the register window is visible.  If not, select an
-     appropriate layout.  */
-  if (TUI_DATA_WIN == NULL || !TUI_DATA_WIN->is_visible ())
-    tui_reg_layout ();
-
   if (group == 0)
     group = general_reggroup;
 
@@ -676,6 +653,21 @@ tui_reg_prev (struct reggroup *current_group, struct gdbarch *gdbarch)
   return group;
 }
 
+/* A helper function to display the register window in the appropriate
+   way.  */
+
+static void
+tui_reg_layout ()
+{
+  enum tui_layout_type cur_layout = tui_current_layout ();
+  enum tui_layout_type new_layout;
+  if (cur_layout == SRC_COMMAND || cur_layout == SRC_DATA_COMMAND)
+    new_layout = SRC_DATA_COMMAND;
+  else
+    new_layout = DISASSEM_DATA_COMMAND;
+  tui_set_layout (new_layout);
+}
+
 /* Implement the 'tui reg' command.  Changes the register group displayed
    in the tui register window.  Displays the tui register window if it is
    not already on display.  */
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 07/14] Change tui_check_register_values to be a method

Tom Tromey-2
In reply to this post by Tom Tromey-2
This changes tui_check_register_values to be a method on
tui_data_window.  An additional check in tui_register_changed is
needed, because TUI_DATA_WIN could be NULL at this point.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-regs.h (struct tui_data_window) <check_register_values>:
        Declare.
        (tui_check_register_values): Don't declare.
        * tui/tui-regs.c (tui_data_window::check_register_values): Rename
        from tui_check_register_values.
        * tui/tui-hooks.c (tui_register_changed): Update.
---
 gdb/ChangeLog       |  9 +++++++++
 gdb/tui/tui-hooks.c |  7 +++++--
 gdb/tui/tui-regs.c  | 29 ++++++++++++-----------------
 gdb/tui/tui-regs.h  |  3 ++-
 4 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 5cc90dd3b6c..2555da7f1ae 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -71,6 +71,9 @@ tui_register_changed (struct frame_info *frame, int regno)
 {
   struct frame_info *fi;
 
+  if (!tui_is_window_visible (DATA_WIN))
+    return;
+
   /* 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
@@ -80,7 +83,7 @@ tui_register_changed (struct frame_info *frame, int regno)
   if (tui_refreshing_registers == 0)
     {
       tui_refreshing_registers = 1;
-      tui_check_register_values (fi);
+      TUI_DATA_WIN->check_register_values (fi);
       tui_refreshing_registers = 0;
     }
 }
@@ -152,7 +155,7 @@ tui_refresh_frame_and_register_information (int registers_too_p)
       && (frame_info_changed_p || registers_too_p))
     {
       tui_refreshing_registers = 1;
-      tui_check_register_values (fi);
+      TUI_DATA_WIN->check_register_values (fi);
       tui_refreshing_registers = 0;
     }
 }
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 147f57a13a2..cd343edc314 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -555,29 +555,24 @@ tui_data_window::refresh_window ()
    given a particular frame.  If the values have changed, they are
    updated with the new value and highlighted.  */
 void
-tui_check_register_values (struct frame_info *frame)
+tui_data_window::check_register_values (struct frame_info *frame)
 {
-  if (TUI_DATA_WIN != NULL
-      && TUI_DATA_WIN->is_visible ())
+  if (regs_content.empty () && display_regs)
+    tui_show_registers (current_group);
+  else
     {
-      if (TUI_DATA_WIN->regs_content.empty ()
-  && TUI_DATA_WIN->display_regs)
- tui_show_registers (TUI_DATA_WIN->current_group);
-      else
+      for (auto &&data_item_win_ptr : regs_content)
  {
-  for (auto &&data_item_win_ptr : TUI_DATA_WIN->regs_content)
-    {
-      int was_hilighted;
+  int was_hilighted;
 
-      was_hilighted = data_item_win_ptr->highlight;
+  was_hilighted = data_item_win_ptr->highlight;
 
-              tui_get_register (frame, data_item_win_ptr.get (),
-                                data_item_win_ptr->item_no,
- &data_item_win_ptr->highlight);
+  tui_get_register (frame, data_item_win_ptr.get (),
+    data_item_win_ptr->item_no,
+    &data_item_win_ptr->highlight);
 
-      if (data_item_win_ptr->highlight || was_hilighted)
- tui_display_register (data_item_win_ptr.get ());
-    }
+  if (data_item_win_ptr->highlight || was_hilighted)
+    tui_display_register (data_item_win_ptr.get ());
  }
     }
 }
diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index b70d8df3620..01c2ea6dad4 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -100,6 +100,8 @@ struct tui_data_window : public tui_win_info
      started from.  If nothing is displayed (-1) is returned.  */
   int display_registers_from_line (int line_no);
 
+  void check_register_values (struct frame_info *frame);
+
 protected:
 
   void do_scroll_vertical (int num_to_scroll) override;
@@ -120,7 +122,6 @@ protected:
   void rerender () override;
 };
 
-extern void tui_check_register_values (struct frame_info *);
 extern void tui_show_registers (struct reggroup *group);
 
 #endif /* TUI_TUI_REGS_H */
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 08/14] Add two methods to tui_data_window

Tom Tromey-2
In reply to this post by Tom Tromey-2
This changes tui_show_registers and tui_show_register_group to be
methods on tui_data_window.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-regs.h (struct tui_data_window) <show_registers,
        show_register_group>: Declare.
        (tui_show_register_group): Don't declare.
        * tui/tui-regs.c (tui_data_window::show_registers): Rename from
        tui_show_registers.
        (tui_data_window::show_register_group): Rename from
        tui_show_register_group.
        (tui_data_window::check_register_values, tui_reg_command):
        Update.
        * tui/tui-layout.c (tui_set_layout): Update.
---
 gdb/ChangeLog        | 13 +++++++++++
 gdb/tui/tui-layout.c |  2 +-
 gdb/tui/tui-regs.c   | 55 ++++++++++++++++++--------------------------
 gdb/tui/tui-regs.h   |  8 +++++--
 4 files changed, 43 insertions(+), 35 deletions(-)

diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index b7c5ed63c64..08b18e5efa1 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -189,7 +189,7 @@ tui_set_layout (enum tui_layout_type layout_type)
       tui_update_source_windows_with_addr (gdbarch, addr);
       if (new_layout == SRC_DATA_COMMAND
   || new_layout == DISASSEM_DATA_COMMAND)
- tui_show_registers (TUI_DATA_WIN->current_group);
+ TUI_DATA_WIN->show_registers (TUI_DATA_WIN->current_group);
     }
 }
 
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index cd343edc314..8adfb1520a2 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -44,11 +44,6 @@
 
 static void tui_display_register (struct tui_data_item_window *data);
 
-static void tui_show_register_group (tui_data_window *win_info,
-     struct reggroup *group,
-     struct frame_info *frame,
-     int refresh_values_only);
-
 /* Get the register from the frame and return a printable
    representation of it.  */
 
@@ -152,33 +147,33 @@ tui_data_window::first_reg_element_no_inline (int line_no) const
 /* Show the registers of the given group in the data window
    and refresh the window.  */
 void
-tui_show_registers (struct reggroup *group)
+tui_data_window::show_registers (struct reggroup *group)
 {
   if (group == 0)
     group = general_reggroup;
 
   /* Say that registers should be displayed, even if there is a
      problem.  */
-  TUI_DATA_WIN->display_regs = true;
+  display_regs = true;
 
   if (target_has_registers && target_has_stack && target_has_memory)
     {
-      tui_show_register_group (TUI_DATA_WIN, group, get_selected_frame (NULL),
-       group == TUI_DATA_WIN->current_group);
+      show_register_group (group, get_selected_frame (NULL),
+   group == current_group);
 
       /* Clear all notation of changed values.  */
-      for (auto &&data_item_win : TUI_DATA_WIN->regs_content)
+      for (auto &&data_item_win : regs_content)
  {
   if (data_item_win != nullptr)
     data_item_win->highlight = false;
  }
-      TUI_DATA_WIN->current_group = group;
-      TUI_DATA_WIN->display_all_data ();
+      current_group = group;
+      display_all_data ();
     }
   else
     {
-      TUI_DATA_WIN->current_group = 0;
-      TUI_DATA_WIN->erase_data_content (_("[ Register Values Unavailable ]"));
+      current_group = 0;
+      erase_data_content (_("[ Register Values Unavailable ]"));
     }
 }
 
@@ -187,22 +182,18 @@ tui_show_registers (struct reggroup *group)
    using the given frame.  Values are refreshed only when
    refresh_values_only is TRUE.  */
 
-static void
-tui_show_register_group (tui_data_window *win_info,
- struct reggroup *group,
-                         struct frame_info *frame,
- int refresh_values_only)
+void
+tui_data_window::show_register_group (struct reggroup *group,
+      struct frame_info *frame,
+      int refresh_values_only)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
   int nr_regs;
   int regnum, pos;
-  char title[80];
 
   /* Make a new title showing which group we display.  */
-  snprintf (title, sizeof (title) - 1, "Register group: %s",
-            reggroup_name (group));
-  xfree (win_info->title);
-  win_info->title = xstrdup (title);
+  xfree (title);
+  title = xstrprintf ("Register group: %s", reggroup_name (group));
 
   /* See how many registers must be displayed.  */
   nr_regs = 0;
@@ -224,14 +215,14 @@ tui_show_register_group (tui_data_window *win_info,
     }
 
   if (!refresh_values_only)
-    win_info->regs_content.clear ();
+    regs_content.clear ();
 
-  if (nr_regs < win_info->regs_content.size ())
-    win_info->regs_content.resize (nr_regs);
+  if (nr_regs < regs_content.size ())
+    regs_content.resize (nr_regs);
   else
     {
-      for (int i = win_info->regs_content.size (); i < nr_regs; ++i)
- win_info->regs_content.emplace_back (new tui_data_item_window ());
+      for (int i = regs_content.size (); i < nr_regs; ++i)
+ regs_content.emplace_back (new tui_data_item_window ());
     }
 
   /* Now set the register names and values.  */
@@ -251,7 +242,7 @@ tui_show_register_group (tui_data_window *win_info,
       if (name == 0 || *name == '\0')
  continue;
 
-      data_item_win = win_info->regs_content[pos].get ();
+      data_item_win = regs_content[pos].get ();
       if (data_item_win)
  {
   if (!refresh_values_only)
@@ -558,7 +549,7 @@ void
 tui_data_window::check_register_values (struct frame_info *frame)
 {
   if (regs_content.empty () && display_regs)
-    tui_show_registers (current_group);
+    show_registers (current_group);
   else
     {
       for (auto &&data_item_win_ptr : regs_content)
@@ -710,7 +701,7 @@ tui_reg_command (const char *args, int from_tty)
       if (match == NULL)
  error (_("unknown register group '%s'"), args);
 
-      tui_show_registers (match);
+      TUI_DATA_WIN->show_registers (match);
     }
   else
     {
diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index 01c2ea6dad4..e07abeda64d 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -102,6 +102,8 @@ struct tui_data_window : public tui_win_info
 
   void check_register_values (struct frame_info *frame);
 
+  void show_registers (struct reggroup *group);
+
 protected:
 
   void do_scroll_vertical (int num_to_scroll) override;
@@ -120,8 +122,10 @@ protected:
   void display_reg_element_at_line (int start_element_no, int start_line_no);
 
   void rerender () override;
-};
 
-extern void tui_show_registers (struct reggroup *group);
+  void show_register_group (struct reggroup *group,
+    struct frame_info *frame,
+    int refresh_values_only);
+};
 
 #endif /* TUI_TUI_REGS_H */
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 09/14] Remove indirection from tui_data_window::regs_content

Tom Tromey-2
In reply to this post by Tom Tromey-2
tui_data_window::regs_content is currently a vector of unique_ptr.
However, due to the way this is managed now, there is no need to keep
the pointers -- it can simply be a vector of the objects themselves.
This patch removes this extra layer of indirection.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-regs.h (struct tui_data_window): Use
        DISABLE_COPY_AND_ASSIGN.
        <regs_content>: Change type, removing unique_ptr.
        <tui_data_window>: Add move constructor.
        * tui/tui-regs.c (tui_data_window::show_registers)
        (tui_data_window::show_register_group)
        (tui_data_window::display_registers_from)
        (tui_data_window::display_registers_from)
        (tui_data_window::first_data_item_displayed)
        (tui_data_window::delete_data_content_windows)
        (tui_data_window::rerender, tui_data_window::refresh_window)
        (tui_data_window::check_register_values): Update.
---
 gdb/ChangeLog      | 15 ++++++++++++++
 gdb/tui/tui-regs.c | 51 ++++++++++++++++------------------------------
 gdb/tui/tui-regs.h |  6 +++++-
 3 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 8adfb1520a2..675e1867d4b 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -163,10 +163,7 @@ tui_data_window::show_registers (struct reggroup *group)
 
       /* Clear all notation of changed values.  */
       for (auto &&data_item_win : regs_content)
- {
-  if (data_item_win != nullptr)
-    data_item_win->highlight = false;
- }
+ data_item_win.highlight = false;
       current_group = group;
       display_all_data ();
     }
@@ -214,16 +211,7 @@ tui_data_window::show_register_group (struct reggroup *group,
       nr_regs++;
     }
 
-  if (!refresh_values_only)
-    regs_content.clear ();
-
-  if (nr_regs < regs_content.size ())
-    regs_content.resize (nr_regs);
-  else
-    {
-      for (int i = regs_content.size (); i < nr_regs; ++i)
- regs_content.emplace_back (new tui_data_item_window ());
-    }
+  regs_content.resize (nr_regs);
 
   /* Now set the register names and values.  */
   pos = 0;
@@ -242,7 +230,7 @@ tui_data_window::show_register_group (struct reggroup *group,
       if (name == 0 || *name == '\0')
  continue;
 
-      data_item_win = regs_content[pos].get ();
+      data_item_win = &regs_content[pos];
       if (data_item_win)
  {
   if (!refresh_values_only)
@@ -273,7 +261,7 @@ tui_data_window::display_registers_from (int start_element_no)
           int len;
 
           len = 0;
-          p = data_item_win->content.get ();
+          p = data_item_win.content.get ();
           if (p != 0)
     len = strlen (p);
 
@@ -301,7 +289,7 @@ tui_data_window::display_registers_from (int start_element_no)
       struct tui_data_item_window *data_item_win;
 
       /* Create the window if necessary.  */
-      data_item_win = regs_content[i].get ();
+      data_item_win = &regs_content[i];
               if (data_item_win->handle != NULL
                   && (data_item_win->height != 1
                       || data_item_win->width != item_win_width
@@ -408,7 +396,7 @@ tui_data_window::first_data_item_displayed ()
     {
       struct tui_gen_win_info *data_item_win;
 
-      data_item_win = regs_content[i].get ();
+      data_item_win = &regs_content[i];
       if (data_item_win->is_visible ())
  return i;
     }
@@ -423,8 +411,8 @@ tui_data_window::delete_data_content_windows ()
 {
   for (auto &&win : regs_content)
     {
-      tui_delete_win (win->handle);
-      win->handle = NULL;
+      tui_delete_win (win.handle);
+      win.handle = NULL;
     }
 }
 
@@ -523,8 +511,8 @@ tui_data_window::rerender ()
   /* Delete all data item windows.  */
   for (auto &&win : regs_content)
     {
-      tui_delete_win (win->handle);
-      win->handle = NULL;
+      tui_delete_win (win.handle);
+      win.handle = NULL;
     }
   display_all_data ();
 }
@@ -536,10 +524,7 @@ tui_data_window::refresh_window ()
 {
   tui_gen_win_info::refresh_window ();
   for (auto &&win : regs_content)
-    {
-      if (win != NULL)
- win->refresh_window ();
-    }
+    win.refresh_window ();
 }
 
 /* This function check all displayed registers for changes in values,
@@ -552,18 +537,18 @@ tui_data_window::check_register_values (struct frame_info *frame)
     show_registers (current_group);
   else
     {
-      for (auto &&data_item_win_ptr : regs_content)
+      for (auto &&data_item_win : regs_content)
  {
   int was_hilighted;
 
-  was_hilighted = data_item_win_ptr->highlight;
+  was_hilighted = data_item_win.highlight;
 
-  tui_get_register (frame, data_item_win_ptr.get (),
-    data_item_win_ptr->item_no,
-    &data_item_win_ptr->highlight);
+  tui_get_register (frame, &data_item_win,
+    data_item_win.item_no,
+    &data_item_win.highlight);
 
-  if (data_item_win_ptr->highlight || was_hilighted)
-    tui_display_register (data_item_win_ptr.get ());
+  if (data_item_win.highlight || was_hilighted)
+    tui_display_register (&data_item_win);
  }
     }
 }
diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index e07abeda64d..24ec587ea2e 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -33,6 +33,10 @@ struct tui_data_item_window : public tui_gen_win_info
   {
   }
 
+  DISABLE_COPY_AND_ASSIGN (tui_data_item_window);
+
+  tui_data_item_window (tui_data_item_window &&) = default;
+
   const char *name = nullptr;
   /* The register number, or data display number.  */
   int item_no = -1;
@@ -60,7 +64,7 @@ struct tui_data_window : public tui_win_info
   }
 
   /* Windows that are used to display registers.  */
-  std::vector<std::unique_ptr<tui_data_item_window>> regs_content;
+  std::vector<tui_data_item_window> regs_content;
   int regs_column_count = 0;
   /* Should regs be displayed at all?  */
   bool display_regs = false;
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 10/14] Remove tui_data_window::display_regs

Tom Tromey-2
In reply to this post by Tom Tromey-2
There's no need for tui_data_window::display_regs any more (if there
ever was).  All the paths through data window construction will end up
setting this to true.  This patch removes the member.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-regs.h (struct tui_data_window) <display_regs>: Remove.
        * tui/tui-regs.c (tui_data_window::show_registers): Update.
        (tui_data_window::check_register_values): Update.
---
 gdb/ChangeLog      | 6 ++++++
 gdb/tui/tui-regs.c | 6 +-----
 gdb/tui/tui-regs.h | 2 --
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 675e1867d4b..f46894075d6 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -152,10 +152,6 @@ tui_data_window::show_registers (struct reggroup *group)
   if (group == 0)
     group = general_reggroup;
 
-  /* Say that registers should be displayed, even if there is a
-     problem.  */
-  display_regs = true;
-
   if (target_has_registers && target_has_stack && target_has_memory)
     {
       show_register_group (group, get_selected_frame (NULL),
@@ -533,7 +529,7 @@ tui_data_window::refresh_window ()
 void
 tui_data_window::check_register_values (struct frame_info *frame)
 {
-  if (regs_content.empty () && display_regs)
+  if (regs_content.empty ())
     show_registers (current_group);
   else
     {
diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index 24ec587ea2e..de445578807 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -66,8 +66,6 @@ struct tui_data_window : public tui_win_info
   /* Windows that are used to display registers.  */
   std::vector<tui_data_item_window> regs_content;
   int regs_column_count = 0;
-  /* Should regs be displayed at all?  */
-  bool display_regs = false;
   struct reggroup *current_group = nullptr;
 
   /* Answer the number of the last line in the regs display.  If there
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 11/14] Change tui_make_window to be a method

Tom Tromey-2
In reply to this post by Tom Tromey-2
I combined several small changes into one patch here.  I believe I
started by noticing that the "title" is not needed by tui_gen_win_info
and could be self-managing (i.e. std::string).  Moving this revealed
that "can_box" is also a property of tui_win_info and not
tui_gen_win_info; and this in turn caused the changes to
tui_make_window and box_win.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-wingeneral.h (tui_make_window): Don't declare.
        * tui/tui-wingeneral.c (box_win): Change type of win_info.
        (box_win): Update.
        (tui_gen_win_info::make_window): Rename from tui_make_window.
        (tui_win_info::make_window): New method.
        (tui_gen_win_info::make_visible): Update.
        * tui/tui-source.c (tui_source_window::set_contents): Update.
        * tui/tui-regs.c (tui_data_window::show_register_group): Update.
        (tui_data_window::display_registers_from): Update.
        * tui/tui-layout.c (tui_gen_win_info::resize): Update.
        * tui/tui-data.h (struct tui_gen_win_info) <make_window>:
        Declare.
        <can_box>: Remove.
        <title>: Remove.
        (struct tui_win_info) <make_window>: Declare.
        <can_box>: Now virtual.
        <title>: New member.
        * tui/tui-data.c (~tui_gen_win_info): Don't free title.
        * tui/tui-command.c (tui_cmd_window::resize): Update.
---
 gdb/ChangeLog            | 22 ++++++++++++++++++++++
 gdb/tui/tui-command.c    |  2 +-
 gdb/tui/tui-data.c       |  1 -
 gdb/tui/tui-data.h       | 17 ++++++++---------
 gdb/tui/tui-layout.c     |  2 +-
 gdb/tui/tui-regs.c       |  5 ++---
 gdb/tui/tui-source.c     |  3 +--
 gdb/tui/tui-wingeneral.c | 31 ++++++++++++++-----------------
 gdb/tui/tui-wingeneral.h |  1 -
 9 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/gdb/tui/tui-command.c b/gdb/tui/tui-command.c
index f2040a75417..62595808cd8 100644
--- a/gdb/tui/tui-command.c
+++ b/gdb/tui/tui-command.c
@@ -54,7 +54,7 @@ tui_cmd_window::resize (int height_, int width_, int origin_x, int origin_y)
   origin.y = origin_y;
 
   if (handle == nullptr)
-    tui_make_window (this);
+    make_window ();
   else
     {
       /* Another reason we don't call the base class method here is
diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
index 9b80aca028e..c11aa43340c 100644
--- a/gdb/tui/tui-data.c
+++ b/gdb/tui/tui-data.c
@@ -243,7 +243,6 @@ tui_win_info::tui_win_info (enum tui_win_type type)
 tui_gen_win_info::~tui_gen_win_info ()
 {
   tui_delete_win (handle);
-  xfree (title);
 }
 
 void
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 0432a53750f..a5ff5e2260e 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -52,6 +52,8 @@ protected:
   {
   }
 
+  virtual void make_window ();
+
 public:
 
   virtual ~tui_gen_win_info ();
@@ -73,12 +75,6 @@ public:
   virtual void resize (int height, int width,
        int origin_x, int origin_y);
 
-  /* Return true if this can be boxed.  */
-  virtual bool can_box () const
-  {
-    return false;
-  }
-
   /* Return true if this window is visible.  */
   bool is_visible () const
   {
@@ -97,8 +93,6 @@ public:
   struct tui_point origin = {0, 0};
   /* Viewport height.  */
   int viewport_height = 0;
-  /* Window title to display.  */
-  char *title = nullptr;
 };
 
 /* Constant definitions.  */
@@ -173,6 +167,8 @@ protected:
 
   void rerender () override;
 
+  void make_window () override;
+
 public:
 
   ~tui_win_info () override
@@ -213,13 +209,16 @@ public:
     return true;
   }
 
-  bool can_box () const override
+  virtual bool can_box () const
   {
     return true;
   }
 
   void check_and_display_highlight_if_needed ();
 
+  /* Window title to display.  */
+  std::string title;
+
   /* Can this window ever be highlighted?  */
   bool can_highlight = true;
 
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 08b18e5efa1..01d50e437e5 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -602,7 +602,7 @@ tui_gen_win_info::resize (int height_, int width_,
     }
 
   if (handle == nullptr)
-    tui_make_window (this);
+    make_window ();
 
   rerender ();
 }
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index f46894075d6..71037d41047 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -185,8 +185,7 @@ tui_data_window::show_register_group (struct reggroup *group,
   int regnum, pos;
 
   /* Make a new title showing which group we display.  */
-  xfree (title);
-  title = xstrprintf ("Register group: %s", reggroup_name (group));
+  title = string_printf ("Register group: %s", reggroup_name (group));
 
   /* See how many registers must be displayed.  */
   nr_regs = 0;
@@ -302,7 +301,7 @@ tui_data_window::display_registers_from (int start_element_no)
   data_item_win->width = item_win_width;
   data_item_win->origin.x = (item_win_width * j) + 1;
   data_item_win->origin.y = cur_y;
-  tui_make_window (data_item_win);
+  data_item_win->make_visible (true);
                   scrollok (data_item_win->handle, FALSE);
  }
               touchwin (data_item_win->handle);
diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
index 07de328bb04..906006a4225 100644
--- a/gdb/tui/tui-source.c
+++ b/gdb/tui/tui-source.c
@@ -153,8 +153,7 @@ tui_source_window::set_contents (struct gdbarch *arch,
     = tui_locator_win_info_ptr ();
   const char *s_filename = symtab_to_filename_for_display (s);
 
-  xfree (title);
-  title = xstrdup (s_filename);
+  title = s_filename;
 
   xfree (fullname);
   fullname = xstrdup (symtab_to_fullname (s));
diff --git a/gdb/tui/tui-wingeneral.c b/gdb/tui/tui-wingeneral.c
index 4e565bdc653..5fa4cfd9eee 100644
--- a/gdb/tui/tui-wingeneral.c
+++ b/gdb/tui/tui-wingeneral.c
@@ -55,7 +55,7 @@ tui_delete_win (WINDOW *window)
 
 /* Draw a border arround the window.  */
 static void
-box_win (struct tui_gen_win_info *win_info,
+box_win (struct tui_win_info *win_info,
  int highlight_flag)
 {
   if (win_info && win_info->handle)
@@ -78,8 +78,8 @@ box_win (struct tui_gen_win_info *win_info,
 #else
       box (win, tui_border_vline, tui_border_hline);
 #endif
-      if (win_info->title)
-        mvwaddstr (win, 0, 3, win_info->title);
+      if (!win_info->title.empty ())
+        mvwaddstr (win, 0, 3, win_info->title.c_str ());
       wattroff (win, attrs);
     }
 }
@@ -126,23 +126,20 @@ tui_win_info::check_and_display_highlight_if_needed ()
 
 
 void
-tui_make_window (struct tui_gen_win_info *win_info)
+tui_gen_win_info::make_window ()
 {
-  WINDOW *handle;
-
-  handle = newwin (win_info->height,
-   win_info->width,
-   win_info->origin.y,
-   win_info->origin.x);
-  win_info->handle = handle;
+  handle = newwin (height, width, origin.y, origin.x);
   if (handle != NULL)
-    {
-      if (win_info->can_box ())
- box_win (win_info, NO_HILITE);
-      scrollok (handle, TRUE);
-    }
+    scrollok (handle, TRUE);
 }
 
+void
+tui_win_info::make_window ()
+{
+  tui_gen_win_info::make_window ();
+  if (handle != NULL && can_box ())
+    box_win (this, NO_HILITE);
+}
 
 /* We can't really make windows visible, or invisible.  So we have to
    delete the entire window when making it visible, and create it
@@ -154,7 +151,7 @@ tui_gen_win_info::make_visible (bool visible)
     return;
 
   if (visible)
-    tui_make_window (this);
+    make_window ();
   else
     {
       tui_delete_win (handle);
diff --git a/gdb/tui/tui-wingeneral.h b/gdb/tui/tui-wingeneral.h
index 6a9de4c9b1c..53d72327b94 100644
--- a/gdb/tui/tui-wingeneral.h
+++ b/gdb/tui/tui-wingeneral.h
@@ -31,7 +31,6 @@ struct tui_gen_win_info;
 extern void tui_make_all_invisible (void);
 
 extern void tui_unhighlight_win (struct tui_win_info *);
-extern void tui_make_window (struct tui_gen_win_info *);
 extern void tui_highlight_win (struct tui_win_info *);
 extern void tui_refresh_all ();
 extern void tui_delete_win (WINDOW *window);
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 12/14] Move some defines to tui-stack.c

Tom Tromey-2
In reply to this post by Tom Tromey-2
Some #defines in tui-data.h are only used in tui-stack.c, so move them
there.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-data.h (PROC_PREFIX, LINE_PREFIX, PC_PREFIX)
        (MIN_LINE_WIDTH, MIN_PROC_WIDTH, MAX_TARGET_WIDTH)
        (MAX_PID_WIDTH): Move to tui-stack.c.
        * tui/tui-stack.c (PROC_PREFIX, LINE_PREFIX, PC_PREFIX)
        (MIN_LINE_WIDTH, MIN_PROC_WIDTH, MAX_TARGET_WIDTH)
        (MAX_PID_WIDTH): Move from tui-data.h.
---
 gdb/ChangeLog       |  9 +++++++++
 gdb/tui/tui-data.h  | 11 -----------
 gdb/tui/tui-stack.c | 12 ++++++++++++
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index a5ff5e2260e..dda15d1fbe1 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -108,19 +108,8 @@ public:
 #define MIN_CMD_WIN_HEIGHT      3
 
 /* Strings to display in the TUI status line.  */
-#define PROC_PREFIX             "In: "
-#define LINE_PREFIX             "L"
-#define PC_PREFIX               "PC: "
 #define SINGLE_KEY              "(SingleKey)"
 
-/* Minimum/Maximum length of some fields displayed in the TUI status
-   line.  */
-#define MIN_LINE_WIDTH     4 /* Use at least 4 digits for line
-   numbers.  */
-#define MIN_PROC_WIDTH    12
-#define MAX_TARGET_WIDTH  10
-#define MAX_PID_WIDTH     19
-
 /* The kinds of layouts available.  */
 enum tui_layout_type
 {
diff --git a/gdb/tui/tui-stack.c b/gdb/tui/tui-stack.c
index 001133c7b5e..0d411331bba 100644
--- a/gdb/tui/tui-stack.c
+++ b/gdb/tui/tui-stack.c
@@ -39,6 +39,18 @@
 
 #include "gdb_curses.h"
 
+#define PROC_PREFIX             "In: "
+#define LINE_PREFIX             "L"
+#define PC_PREFIX               "PC: "
+
+/* Minimum/Maximum length of some fields displayed in the TUI status
+   line.  */
+#define MIN_LINE_WIDTH     4 /* Use at least 4 digits for line
+   numbers.  */
+#define MIN_PROC_WIDTH    12
+#define MAX_TARGET_WIDTH  10
+#define MAX_PID_WIDTH     19
+
 static struct tui_locator_window _locator;
 
 /* Get a printable name for the function at the address.
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 13/14] Remove some defines from tui-data.h

Tom Tromey-2
In reply to this post by Tom Tromey-2
This removes the HILITE and NO_HILITE defines from tui-data.h, in
favor of simply passing a bool to box_win.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-wingeneral.c (box_win): Change type of highlight_flag.
        (tui_unhighlight_win, tui_highlight_win)
        (tui_win_info::make_window): Update.
        * tui/tui-data.h (HILITE, NO_HILITE): Remove.
---
 gdb/ChangeLog            |  7 +++++++
 gdb/tui/tui-data.h       |  2 --
 gdb/tui/tui-wingeneral.c | 10 +++++-----
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index dda15d1fbe1..6dfea41d49e 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -102,8 +102,6 @@ public:
 #define CMD_NAME                "cmd"
 #define DATA_NAME               "regs"
 #define DISASSEM_NAME           "asm"
-#define HILITE                  TRUE
-#define NO_HILITE               FALSE
 #define MIN_WIN_HEIGHT          3
 #define MIN_CMD_WIN_HEIGHT      3
 
diff --git a/gdb/tui/tui-wingeneral.c b/gdb/tui/tui-wingeneral.c
index 5fa4cfd9eee..ab0363f856c 100644
--- a/gdb/tui/tui-wingeneral.c
+++ b/gdb/tui/tui-wingeneral.c
@@ -56,7 +56,7 @@ tui_delete_win (WINDOW *window)
 /* Draw a border arround the window.  */
 static void
 box_win (struct tui_win_info *win_info,
- int highlight_flag)
+ bool highlight_flag)
 {
   if (win_info && win_info->handle)
     {
@@ -64,7 +64,7 @@ box_win (struct tui_win_info *win_info,
       int attrs;
 
       win = win_info->handle;
-      if (highlight_flag == HILITE)
+      if (highlight_flag)
         attrs = tui_active_border_attrs;
       else
         attrs = tui_border_attrs;
@@ -92,7 +92,7 @@ tui_unhighlight_win (struct tui_win_info *win_info)
       && win_info->can_highlight
       && win_info->handle != NULL)
     {
-      box_win (win_info, NO_HILITE);
+      box_win (win_info, false);
       win_info->refresh_window ();
       win_info->set_highlight (false);
     }
@@ -106,7 +106,7 @@ tui_highlight_win (struct tui_win_info *win_info)
       && win_info->can_highlight
       && win_info->handle != NULL)
     {
-      box_win (win_info, HILITE);
+      box_win (win_info, true);
       win_info->refresh_window ();
       win_info->set_highlight (true);
     }
@@ -138,7 +138,7 @@ tui_win_info::make_window ()
 {
   tui_gen_win_info::make_window ();
   if (handle != NULL && can_box ())
-    box_win (this, NO_HILITE);
+    box_win (this, false);
 }
 
 /* We can't really make windows visible, or invisible.  So we have to
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 14/14] Change some tui_data_window methods to be private

Tom Tromey-2
In reply to this post by Tom Tromey-2
Turning various calls into methods has made it possible to now change
some tui_data_window methods to be private.

gdb/ChangeLog
2019-08-18  Tom Tromey  <[hidden email]>

        * tui/tui-regs.h (struct tui_data_window) <last_regs_line_no,
        line_from_reg_element_no, first_reg_element_no_inline,
        display_all_data, delete_data_content_windows,
        erase_data_content>: Now private.
---
 gdb/ChangeLog      |  7 ++++++
 gdb/tui/tui-regs.h | 62 ++++++++++++++++++++++++----------------------
 2 files changed, 39 insertions(+), 30 deletions(-)

diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index de445578807..95e944038c7 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -68,28 +68,20 @@ struct tui_data_window : public tui_win_info
   int regs_column_count = 0;
   struct reggroup *current_group = nullptr;
 
-  /* Answer the number of the last line in the regs display.  If there
-     are no registers (-1) is returned.  */
-  int last_regs_line_no () const;
+  void check_register_values (struct frame_info *frame);
 
-  /* Answer the line number that the register element at element_no is
-     on.  If element_no is greater than the number of register
-     elements there are, -1 is returned.  */
-  int line_from_reg_element_no (int element_no) const;
+  void show_registers (struct reggroup *group);
 
-  /* Answer the index of the first element in line_no.  If line_no is
-     past the register area (-1) is returned.  */
-  int first_reg_element_no_inline (int line_no) const;
+protected:
 
-  /* Displays the data that is in the data window's content.  It does
-     not set the content.  */
-  void display_all_data ();
+  void do_scroll_vertical (int num_to_scroll) override;
+  void do_scroll_horizontal (int num_to_scroll) override
+  {
+  }
 
-  /* Delete all the item windows in the data window.  This is usually
-     done when the data window is scrolled.  */
-  void delete_data_content_windows ();
+  void rerender () override;
 
-  void erase_data_content (const char *prompt);
+private:
 
   /* Display the registers in the content from 'start_element_no'
      until the end of the register content or the end of the display
@@ -102,17 +94,6 @@ struct tui_data_window : public tui_win_info
      started from.  If nothing is displayed (-1) is returned.  */
   int display_registers_from_line (int line_no);
 
-  void check_register_values (struct frame_info *frame);
-
-  void show_registers (struct reggroup *group);
-
-protected:
-
-  void do_scroll_vertical (int num_to_scroll) override;
-  void do_scroll_horizontal (int num_to_scroll) override
-  {
-  }
-
   /* Return the index of the first element displayed.  If none are
      displayed, then return -1.  */
   int first_data_item_displayed ();
@@ -123,11 +104,32 @@ protected:
      display off the end of the register display.  */
   void display_reg_element_at_line (int start_element_no, int start_line_no);
 
-  void rerender () override;
-
   void show_register_group (struct reggroup *group,
     struct frame_info *frame,
     int refresh_values_only);
+
+  /* Answer the number of the last line in the regs display.  If there
+     are no registers (-1) is returned.  */
+  int last_regs_line_no () const;
+
+  /* Answer the line number that the register element at element_no is
+     on.  If element_no is greater than the number of register
+     elements there are, -1 is returned.  */
+  int line_from_reg_element_no (int element_no) const;
+
+  /* Answer the index of the first element in line_no.  If line_no is
+     past the register area (-1) is returned.  */
+  int first_reg_element_no_inline (int line_no) const;
+
+  /* Displays the data that is in the data window's content.  It does
+     not set the content.  */
+  void display_all_data ();
+
+  /* Delete all the item windows in the data window.  This is usually
+     done when the data window is scrolled.  */
+  void delete_data_content_windows ();
+
+  void erase_data_content (const char *prompt);
 };
 
 #endif /* TUI_TUI_REGS_H */
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 05/14] Change tui_data_item_window::content to be a unique_xmalloc_ptr

Pedro Alves-7
In reply to this post by Tom Tromey-2
On 8/18/19 6:27 PM, Tom Tromey wrote:

> @@ -309,19 +301,13 @@ tui_data_window::display_registers_from (int start_element_no)
>        int max_len = 0;
>        for (auto &&data_item_win : regs_content)
>          {
> -          char *p;
> +          const char *p;
>            int len;
>  
>            len = 0;
> -          p = data_item_win->content;
> +          p = data_item_win->content.get ();
>            if (p != 0)
> -            while (*p)
> -              {
> -                if (*p++ == '\t')
> -                  len = 8 * ((len / 8) + 1);
> -                else
> -                  len++;
> -              }
> +    len = strlen (p);
>  

Is this related?  

(Also, I didn't bother to think through whether strlen
is equivalent to the old code.  Is it?)

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 00/14] TUI refactoring round N

Pedro Alves-7
In reply to this post by Tom Tromey-2
On 8/18/19 6:27 PM, Tom Tromey wrote:
> Here's yet another series of TUI refactorings.  This series should
> have no user-visible changes.  There are several more series like this
> to come, but as before, they are interspersed with user-visible
> patches which I plan to submit separately.
>
> Each patch was built and tested using gdb.tui on x86-64 Fedora 28.

Other than the comment to patch #5, this all looks good to me.

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 05/14] Change tui_data_item_window::content to be a unique_xmalloc_ptr

Tom Tromey-2
In reply to this post by Pedro Alves-7
>>>>> "Pedro" == Pedro Alves <[hidden email]> writes:

Pedro> On 8/18/19 6:27 PM, Tom Tromey wrote:

>> @@ -309,19 +301,13 @@ tui_data_window::display_registers_from (int start_element_no)
>> int max_len = 0;
>> for (auto &&data_item_win : regs_content)
>> {
>> -          char *p;
>> +          const char *p;
>> int len;
>>
>> len = 0;
>> -          p = data_item_win->content;
>> +          p = data_item_win->content.get ();
>> if (p != 0)
>> -            while (*p)
>> -              {
>> -                if (*p++ == '\t')
>> -                  len = 8 * ((len / 8) + 1);
>> -                else
>> -                  len++;
>> -              }
>> +    len = strlen (p);
>>

Pedro> Is this related?  

Yeah, though it isn't obvious.  I will amend the commit message to make
this clear.

What is happening here is that the content can only be computed by
tui_register_format, which calls tui_expand_tabs before returning.  So,
it's not possible to see a tab in the contents.

Pedro> (Also, I didn't bother to think through whether strlen
Pedro> is equivalent to the old code.  Is it?)

I think so.

Tom