diff mbox series

[05/15] gdb/tui: convert if/error to an assert

Message ID 41ae74a51377abd813ad2bdb29cedfbfbdc54e3c.1673000632.git.aburgess@redhat.com
State Committed
Commit 6789344ab22f7fbce94a31297b994f85685b90c6
Headers
Series Mixed bag of TUI tests and fixes |

Commit Message

Andrew Burgess Jan. 6, 2023, 10:25 a.m. UTC 
  While working on the previous commit, I realised that there was an
error in tui_set_focus_command that could never be triggered.

Since the big tui rewrite (adding dynamic layouts) it is no longer
true that there is a tui_win_info object for every window at all
times.  We now only create a tui_win_info object for a particular
window, when the window is part of the current layout.  As a result,
if we have a tui_win_info pointer, then the window must be visible
inside tui_set_focus_command (this function calls tui_enable as its
first action, which makes the current layout visible).

The gdb.tui/tui-focus.exp test script exercises this area of code, and
doesn't trigger the assert, nor do any of our other existing tui
tests.
---
 gdb/tui/tui-win.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Andrew Burgess Jan. 25, 2023, 11:33 a.m. UTC | #1 
Andrew Burgess <aburgess@redhat.com> writes:

> While working on the previous commit, I realised that there was an
> error in tui_set_focus_command that could never be triggered.
>
> Since the big tui rewrite (adding dynamic layouts) it is no longer
> true that there is a tui_win_info object for every window at all
> times.  We now only create a tui_win_info object for a particular
> window, when the window is part of the current layout.  As a result,
> if we have a tui_win_info pointer, then the window must be visible
> inside tui_set_focus_command (this function calls tui_enable as its
> first action, which makes the current layout visible).
>
> The gdb.tui/tui-focus.exp test script exercises this area of code, and
> doesn't trigger the assert, nor do any of our other existing tui
> tests.

I've gone ahead and pushed this patch.  The final version is included
below.

Thanks,
Andrew

---

commit 6789344ab22f7fbce94a31297b994f85685b90c6
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Dec 22 12:43:38 2022 +0000

    gdb/tui: convert if/error to an assert
    
    While working on the previous commit, I realised that there was an
    error in tui_set_focus_command that could never be triggered.
    
    Since the big tui rewrite (adding dynamic layouts) it is no longer
    true that there is a tui_win_info object for every window at all
    times.  We now only create a tui_win_info object for a particular
    window, when the window is part of the current layout.  As a result,
    if we have a tui_win_info pointer, then the window must be visible
    inside tui_set_focus_command (this function calls tui_enable as its
    first action, which makes the current layout visible).
    
    The gdb.tui/tui-focus.exp test script exercises this area of code, and
    doesn't trigger the assert, nor do any of our other existing tui
    tests.

diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 9c088899817..4fc8e7a4503 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -720,8 +720,11 @@ tui_set_focus_command (const char *arg, int from_tty)
 
   if (win_info == NULL)
     error (_("Unrecognized window name \"%s\""), arg);
-  if (!win_info->is_visible ())
-    error (_("Window \"%s\" is not visible"), arg);
+
+  /* If a window is part of the current layout then it will have a
+     tui_win_info associated with it and be visible, otherwise, there will
+     be no tui_win_info and the above error will have been raised.  */
+  gdb_assert (win_info->is_visible ());
 
   if (!win_info->can_focus ())
     error (_("Window \"%s\" cannot be focused"), arg);
diff mbox series

Patch

diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 9c088899817..4fc8e7a4503 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -720,8 +720,11 @@  tui_set_focus_command (const char *arg, int from_tty)
 
   if (win_info == NULL)
     error (_("Unrecognized window name \"%s\""), arg);
-  if (!win_info->is_visible ())
-    error (_("Window \"%s\" is not visible"), arg);
+
+  /* If a window is part of the current layout then it will have a
+     tui_win_info associated with it and be visible, otherwise, there will
+     be no tui_win_info and the above error will have been raised.  */
+  gdb_assert (win_info->is_visible ());
 
   if (!win_info->can_focus ())
     error (_("Window \"%s\" cannot be focused"), arg);