[PATCH 1/3] Remove superfluous function key_is_command_char()

[PATCH 1/3] Remove superfluous function key_is_command_char()

[Patrick Palka]

The function key_is_command_char() is simply a predicate that determines
whether the function tui_dispatch_ctrl_char() will do anything useful.
Since tui_dispatch_ctrl_char() performs the same checks as
key_is_command_char() it is unnecessary to keep key_is_command_char()
around.  This patch removes this useless function and instead
unconditionally calls tui_dispatch_ctrl_char() inside its only caller,
tui_getc().

gdb/ChangeLog:

	* tui/tui-io.c (tui_getc): Don't call key_is_command_char.
	(key_is_command_char): Delete.
---
 gdb/tui/tui-io.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 233e7a6..b0e6c75 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -63,17 +63,6 @@ key_is_backspace (int ch)
   return (ch == 8);
 }
 
-int
-key_is_command_char (int ch)
-{
-  return ((ch == KEY_NPAGE) || (ch == KEY_PPAGE)
-	  || (ch == KEY_LEFT) || (ch == KEY_RIGHT)
-	  || (ch == KEY_UP) || (ch == KEY_DOWN)
-	  || (ch == KEY_SF) || (ch == KEY_SR)
-	  || (ch == (int)'\f') 
-	  || key_is_start_sequence (ch));
-}
-
 /* Use definition from readline 4.3.  */
 #undef CTRL_CHAR
 #define CTRL_CHAR(c) \
@@ -682,10 +671,8 @@ tui_getc (FILE *fp)
         }
     }
   
-  if (key_is_command_char (ch))
-    {				/* Handle prev/next/up/down here.  */
-      ch = tui_dispatch_ctrl_char (ch);
-    }
+  /* Handle prev/next/up/down here.  */
+  ch = tui_dispatch_ctrl_char (ch);
   
   if (ch == '\n' || ch == '\r' || ch == '\f')
     TUI_CMD_WIN->detail.command_info.curch = 0;
-- 
2.2.1.212.gc5b9256

[PATCH 2/3] TUI: Don't print KEY_RESIZE keys

[Patrick Palka]

wgetch() sometimes returns KEY_RESIZE when ncurses detects that the
terminal has been resized.  This character then gets passed on to
readline which interprets it as a multibyte character.  Instead of
passing on this character to readline, intercept it as a (no-op) control
character and pass '\0' to readline.

gdb/ChangeLog:

	* tui/tui-command.c (tui_dispatch_ctrl_char): Ignore KEY_RESIZE
	keys.
---
 gdb/tui/tui-command.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gdb/tui/tui-command.c b/gdb/tui/tui-command.c
index d1a5ddb..dfcf512 100644
--- a/gdb/tui/tui-command.c
+++ b/gdb/tui/tui-command.c
@@ -121,6 +121,7 @@ tui_dispatch_ctrl_char (unsigned int ch)
 	  tui_scroll_right (win_info, 1);
 	  break;
 	case '\f':
+	case KEY_RESIZE:
           break;
 	default:
 	  c = ch_copy;
-- 
2.2.1.212.gc5b9256

Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys

[Pedro Alves]

On 01/08/2015 04:04 AM, Patrick Palka wrote:
> wgetch() sometimes returns KEY_RESIZE when ncurses detects that the
> terminal has been resized.  

I think curses SIGWINCH handler ends up _not_ installed, right?
We install our own, and so does readline.
So how did a resize manage to be detected/processed while inside
wgetch?

Thanks,
Pedro Alves

Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys

[Patrick Palka]

On Thu, Jan 8, 2015 at 6:29 AM, Pedro Alves <palves@redhat.com> wrote:
> On 01/08/2015 04:04 AM, Patrick Palka wrote:
>> wgetch() sometimes returns KEY_RESIZE when ncurses detects that the
>> terminal has been resized.
>
> I think curses SIGWINCH handler ends up _not_ installed, right?
> We install our own, and so does readline.
> So how did a resize manage to be detected/processed while inside
> wgetch?

I'm pretty sure that the SIGWINCH handlers does not get installed.
However ncurses may detect a resize event when we exit TUI (exiting
ncurses), then resize the terminal, then re-enter TUI (restarting
ncurses).  From there a KEY_RESIZE key is added to its internal FIFO.
And the next call to wgetch will return this KEY_RESIZE key.

>
> Thanks,
> Pedro Alves
>

Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys

[Pedro Alves]

On 01/08/2015 12:32 PM, Patrick Palka wrote:
> On Thu, Jan 8, 2015 at 6:29 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 01/08/2015 04:04 AM, Patrick Palka wrote:
>>> wgetch() sometimes returns KEY_RESIZE when ncurses detects that the
>>> terminal has been resized.
>>
>> I think curses SIGWINCH handler ends up _not_ installed, right?
>> We install our own, and so does readline.
>> So how did a resize manage to be detected/processed while inside
>> wgetch?
> 
> I'm pretty sure that the SIGWINCH handlers does not get installed.
> However ncurses may detect a resize event when we exit TUI (exiting
> ncurses), then resize the terminal, then re-enter TUI (restarting
> ncurses).  From there a KEY_RESIZE key is added to its internal FIFO.
> And the next call to wgetch will return this KEY_RESIZE key.

Doesn't that mean then that we're delaying the resize until it's
"too late"?  IOW, there's a moment where we show the contents with
the wrong sizes.  And trying that out, I do see that happen: if I
disable the TUI, resize the terminal, and then reenable the TUI,
like you say, then until you press _another_ key, the windows
have the wrong size.  We should have resized them already when we
re-entered the TUI.

And if I try that when I'm already debugging an inferior, stopped at
a breakpoint, I see nasty things.  If I reduce the size of the
window, TUI gets stuck in an infinite loop in tui_show_source_line:

(top-gdb) bt
#0  0x0000003b46008446 in waddch_literal (win=0x28dfd30, ch=32) at ../../ncurses/base/lib_addch.c:253
#1  0x0000003b4600859c in waddch_nosync (win=0x28dfd30, ch=32) at ../../ncurses/base/lib_addch.c:437
#2  0x0000003b46008878 in waddch (win=0x28dfd30, ch=32) at ../../ncurses/base/lib_addch.c:543
#3  0x0000000000524d08 in tui_show_source_line (win_info=0x28dfc90, lineno=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:292
#4  0x0000000000524d8f in tui_show_source_content (win_info=0x28dfc90) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:305
#5  0x00000000005220cb in tui_refresh_all_win () at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:633
#6  0x000000000052611c in tui_enable () at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:486
#7  0x0000000000525861 in tui_rl_switch_mode (notused1=1, notused2=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:108
#8  0x0000000000796d6f in _rl_dispatch_subseq (key=1, map=0xd585c0 <emacs_ctlx_keymap>, got_subseq=0) at /home/pedro/gdb/mygit/src/readline/readline.c:774
#9  0x0000000000796acb in _rl_dispatch_callback (cxt=0x26e6500) at /home/pedro/gdb/mygit/src/readline/readline.c:686
#10 0x00000000007af20b in rl_callback_read_char () at /home/pedro/gdb/mygit/src/readline/callback.c:170
#11 0x000000000063cefd in rl_callback_read_char_wrapper (client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:171
#12 0x000000000063d366 in stdin_event_handler (error=0, client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:432
#13 0x000000000063bec6 in handle_file_event (data=...) at /home/pedro/gdb/mygit/src/gdb/event-loop.c:762
---Type <return> to continue, or q <return> to quit---
#14 0x000000000063b3ad in process_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:339
#15 0x000000000063b474 in gdb_do_one_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:403
#16 0x000000000063b4c4 in start_event_loop () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:428
#17 0x000000000063cf2f in cli_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:186
#18 0x0000000000633284 in current_interp_command_loop () at /home/pedro/gdb/mygit/src/gdb/interps.c:317
#19 0x000000000063444d in captured_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/main.c:321
#20 0x0000000000630141 in catch_errors (func=0x634432 <captured_command_loop>, func_args=0x0, errstring=0x9422e1 "", mask=RETURN_MASK_ALL)
    at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
#21 0x000000000063594b in captured_main (data=0x7fffe05ca000) at /home/pedro/gdb/mygit/src/gdb/main.c:1149
#22 0x0000000000630141 in catch_errors (func=0x63484a <captured_main>, func_args=0x7fffe05ca000, errstring=0x9422e1 "", mask=RETURN_MASK_ALL)
    at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
#23 0x0000000000635974 in gdb_main (args=0x7fffe05ca000) at /home/pedro/gdb/mygit/src/gdb/main.c:1157
#24 0x0000000000464e57 in main (argc=3, argv=0x7fffe05ca108) at /home/pedro/gdb/mygit/src/gdb/gdb.c:32



If instead I increase the size of the window, GDB aborts.  E.g.,:

(top-gdb) bt
#0  internal_error (file=0x986408 "/home/pedro/gdb/mygit/src/gdb/utils.c", line=1076, fmt=0x986834 "virtual memory exhausted.")
    at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
#1  0x000000000074b85d in malloc_failure (size=-8) at /home/pedro/gdb/mygit/src/gdb/utils.c:1076
#2  0x0000000000783616 in xmalloc (size=18446744073709551608) at /home/pedro/gdb/mygit/src/gdb/common/common-utils.c:43
#3  0x000000000051a13b in tui_alloc_content (num_elements=-1, type=EXEC_INFO_WIN) at /home/pedro/gdb/mygit/src/gdb/tui/tui-data.c:590
#4  0x00000000005253ee in tui_set_exec_info_content (win_info=0x1a4a6c0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:510
#5  0x0000000000525672 in tui_update_exec_info (win_info=0x1a4a6c0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:602
#6  0x00000000005221c8 in tui_refresh_all_win () at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:636
#7  0x000000000051d3b1 in tui_handle_resize_during_io (original_ch=410) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:770
#8  0x000000000051d251 in tui_getc (fp=0x3b36db8640 <_IO_2_1_stdin_>) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:693
#9  0x00000000007aed93 in rl_read_key () at /home/pedro/gdb/mygit/src/readline/input.c:448
#10 0x0000000000796848 in readline_internal_char () at /home/pedro/gdb/mygit/src/readline/readline.c:517
#11 0x00000000007af375 in rl_callback_read_char () at /home/pedro/gdb/mygit/src/readline/callback.c:201
#12 0x000000000063cfb9 in rl_callback_read_char_wrapper (client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:171
#13 0x000000000063d422 in stdin_event_handler (error=0, client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:432

Another time:

(top-gdb) bt
#0  internal_error (file=0x986408 "/home/pedro/gdb/mygit/src/gdb/utils.c", line=1076, fmt=0x986834 "virtual memory exhausted.")
    at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
#1  0x000000000074b85d in malloc_failure (size=-79) at /home/pedro/gdb/mygit/src/gdb/utils.c:1076
#2  0x0000000000783616 in xmalloc (size=18446744073709551537) at /home/pedro/gdb/mygit/src/gdb/common/common-utils.c:43
#3  0x00000000005256c5 in tui_alloc_source_buffer (win_info=0x28abee0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:621
#4  0x000000000051ff7b in tui_set_source_content (s=0x2299ae0, line_no=1, noerror=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui-source.c:53
#5  0x0000000000524871 in tui_update_source_window_as_is (win_info=0x28abee0, gdbarch=0x26a2450, s=0x2299ae0, line_or_addr=..., noerror=1)
    at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:96
#6  0x0000000000524824 in tui_update_source_window (win_info=0x28abee0, gdbarch=0x26a2450, s=0x2299ae0, line_or_addr=..., noerror=1)
    at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:78
#7  0x000000000052360a in make_visible_with_new_height (win_info=0x28abee0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:1381
#8  0x000000000052242c in tui_resize_all () at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:723
#9  0x000000000051d3ac in tui_handle_resize_during_io (original_ch=410) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:769
#10 0x000000000051d251 in tui_getc (fp=0x3b36db8640 <_IO_2_1_stdin_>) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:693
#11 0x00000000007aed93 in rl_read_key () at /home/pedro/gdb/mygit/src/readline/input.c:448
#12 0x0000000000796848 in readline_internal_char () at /home/pedro/gdb/mygit/src/readline/readline.c:517
#13 0x00000000007af375 in rl_callback_read_char () at /home/pedro/gdb/mygit/src/readline/callback.c:201
#14 0x000000000063cfb9 in rl_callback_read_char_wrapper (client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:171
#15 0x000000000063d422 in stdin_event_handler (error=0, client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:432
#16 0x000000000063bf82 in handle_file_event (data=...) at /home/pedro/gdb/mygit/src/gdb/event-loop.c:762
#17 0x000000000063b469 in process_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:339
#18 0x000000000063b530 in gdb_do_one_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:403
#19 0x000000000063b580 in start_event_loop () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:428
#20 0x000000000063cfeb in cli_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:186
#21 0x0000000000633340 in current_interp_command_loop () at /home/pedro/gdb/mygit/src/gdb/interps.c:317
#22 0x0000000000634509 in captured_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/main.c:321
#23 0x00000000006301fd in catch_errors (func=0x6344ee <captured_command_loop>, func_args=0x0, errstring=0x942261 "", mask=RETURN_MASK_ALL)
    at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
---Type <return> to continue, or q <return> to quit---
#24 0x0000000000635a07 in captured_main (data=0x7fffeab9cbb0) at /home/pedro/gdb/mygit/src/gdb/main.c:1149
#25 0x00000000006301fd in catch_errors (func=0x634906 <captured_main>, func_args=0x7fffeab9cbb0, errstring=0x942261 "", mask=RETURN_MASK_ALL)
    at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
#26 0x0000000000635a30 in gdb_main (args=0x7fffeab9cbb0) at /home/pedro/gdb/mygit/src/gdb/main.c:1157
#27 0x0000000000464e87 in main (argc=3, argv=0x7fffeab9ccb8) at /home/pedro/gdb/mygit/src/gdb/gdb.c:32

Do you see these?

As a quick try, I added a call to tui_resize_all in tui_enable, and
it seems to fix these, and almost fix the "stale" size when we
reenable the TUI, but a subsequent keypress still changes the
layout, so apparently something more should be done.

diff --git c/gdb/tui/tui.c w/gdb/tui/tui.c
index 00e505d..f3d0fbf 100644
--- c/gdb/tui/tui.c
+++ w/gdb/tui/tui.c
@@ -467,6 +467,7 @@ tui_enable (void)
         Curses will restore this state when endwin() is called.  */
      def_shell_mode ();
      clearok (stdscr, TRUE);
+     tui_resize_all ();
    }

   /* Install the TUI specific hooks.  */

Thanks,
Pedro Alves

Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys

[Patrick Palka]

On Thu, Jan 8, 2015 at 8:22 AM, Pedro Alves <palves@redhat.com> wrote:
> On 01/08/2015 12:32 PM, Patrick Palka wrote:
>> On Thu, Jan 8, 2015 at 6:29 AM, Pedro Alves <palves@redhat.com> wrote:
>>> On 01/08/2015 04:04 AM, Patrick Palka wrote:
>>>> wgetch() sometimes returns KEY_RESIZE when ncurses detects that the
>>>> terminal has been resized.
>>>
>>> I think curses SIGWINCH handler ends up _not_ installed, right?
>>> We install our own, and so does readline.
>>> So how did a resize manage to be detected/processed while inside
>>> wgetch?
>>
>> I'm pretty sure that the SIGWINCH handlers does not get installed.
>> However ncurses may detect a resize event when we exit TUI (exiting
>> ncurses), then resize the terminal, then re-enter TUI (restarting
>> ncurses).  From there a KEY_RESIZE key is added to its internal FIFO.
>> And the next call to wgetch will return this KEY_RESIZE key.
>
> Doesn't that mean then that we're delaying the resize until it's
> "too late"?  IOW, there's a moment where we show the contents with
> the wrong sizes.  And trying that out, I do see that happen: if I
> disable the TUI, resize the terminal, and then reenable the TUI,
> like you say, then until you press _another_ key, the windows
> have the wrong size.  We should have resized them already when we
> re-entered the TUI.

I think that is a separate issue.  ncurses simply detects that a
resize happened but doesn't do anything about it, IIRC.  It is up to
the program to intercept these KEY_RESIZEs and to make sure the screen
is resized correctly.  This patch makes tui_getc() ignore such keys
because they are not reliable without a corresponding SIGWINCH
handler.

>
> And if I try that when I'm already debugging an inferior, stopped at
> a breakpoint, I see nasty things.  If I reduce the size of the
> window, TUI gets stuck in an infinite loop in tui_show_source_line:
>
> (top-gdb) bt
> #0  0x0000003b46008446 in waddch_literal (win=0x28dfd30, ch=32) at ../../ncurses/base/lib_addch.c:253
> #1  0x0000003b4600859c in waddch_nosync (win=0x28dfd30, ch=32) at ../../ncurses/base/lib_addch.c:437
> #2  0x0000003b46008878 in waddch (win=0x28dfd30, ch=32) at ../../ncurses/base/lib_addch.c:543
> #3  0x0000000000524d08 in tui_show_source_line (win_info=0x28dfc90, lineno=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:292
> #4  0x0000000000524d8f in tui_show_source_content (win_info=0x28dfc90) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:305
> #5  0x00000000005220cb in tui_refresh_all_win () at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:633
> #6  0x000000000052611c in tui_enable () at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:486
> #7  0x0000000000525861 in tui_rl_switch_mode (notused1=1, notused2=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:108
> #8  0x0000000000796d6f in _rl_dispatch_subseq (key=1, map=0xd585c0 <emacs_ctlx_keymap>, got_subseq=0) at /home/pedro/gdb/mygit/src/readline/readline.c:774
> #9  0x0000000000796acb in _rl_dispatch_callback (cxt=0x26e6500) at /home/pedro/gdb/mygit/src/readline/readline.c:686
> #10 0x00000000007af20b in rl_callback_read_char () at /home/pedro/gdb/mygit/src/readline/callback.c:170
> #11 0x000000000063cefd in rl_callback_read_char_wrapper (client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:171
> #12 0x000000000063d366 in stdin_event_handler (error=0, client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:432
> #13 0x000000000063bec6 in handle_file_event (data=...) at /home/pedro/gdb/mygit/src/gdb/event-loop.c:762
> ---Type <return> to continue, or q <return> to quit---
> #14 0x000000000063b3ad in process_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:339
> #15 0x000000000063b474 in gdb_do_one_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:403
> #16 0x000000000063b4c4 in start_event_loop () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:428
> #17 0x000000000063cf2f in cli_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:186
> #18 0x0000000000633284 in current_interp_command_loop () at /home/pedro/gdb/mygit/src/gdb/interps.c:317
> #19 0x000000000063444d in captured_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/main.c:321
> #20 0x0000000000630141 in catch_errors (func=0x634432 <captured_command_loop>, func_args=0x0, errstring=0x9422e1 "", mask=RETURN_MASK_ALL)
>     at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
> #21 0x000000000063594b in captured_main (data=0x7fffe05ca000) at /home/pedro/gdb/mygit/src/gdb/main.c:1149
> #22 0x0000000000630141 in catch_errors (func=0x63484a <captured_main>, func_args=0x7fffe05ca000, errstring=0x9422e1 "", mask=RETURN_MASK_ALL)
>     at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
> #23 0x0000000000635974 in gdb_main (args=0x7fffe05ca000) at /home/pedro/gdb/mygit/src/gdb/main.c:1157
> #24 0x0000000000464e57 in main (argc=3, argv=0x7fffe05ca108) at /home/pedro/gdb/mygit/src/gdb/gdb.c:32
>
>
>
> If instead I increase the size of the window, GDB aborts.  E.g.,:
>
> (top-gdb) bt
> #0  internal_error (file=0x986408 "/home/pedro/gdb/mygit/src/gdb/utils.c", line=1076, fmt=0x986834 "virtual memory exhausted.")
>     at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
> #1  0x000000000074b85d in malloc_failure (size=-8) at /home/pedro/gdb/mygit/src/gdb/utils.c:1076
> #2  0x0000000000783616 in xmalloc (size=18446744073709551608) at /home/pedro/gdb/mygit/src/gdb/common/common-utils.c:43
> #3  0x000000000051a13b in tui_alloc_content (num_elements=-1, type=EXEC_INFO_WIN) at /home/pedro/gdb/mygit/src/gdb/tui/tui-data.c:590
> #4  0x00000000005253ee in tui_set_exec_info_content (win_info=0x1a4a6c0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:510
> #5  0x0000000000525672 in tui_update_exec_info (win_info=0x1a4a6c0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:602
> #6  0x00000000005221c8 in tui_refresh_all_win () at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:636
> #7  0x000000000051d3b1 in tui_handle_resize_during_io (original_ch=410) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:770
> #8  0x000000000051d251 in tui_getc (fp=0x3b36db8640 <_IO_2_1_stdin_>) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:693
> #9  0x00000000007aed93 in rl_read_key () at /home/pedro/gdb/mygit/src/readline/input.c:448
> #10 0x0000000000796848 in readline_internal_char () at /home/pedro/gdb/mygit/src/readline/readline.c:517
> #11 0x00000000007af375 in rl_callback_read_char () at /home/pedro/gdb/mygit/src/readline/callback.c:201
> #12 0x000000000063cfb9 in rl_callback_read_char_wrapper (client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:171
> #13 0x000000000063d422 in stdin_event_handler (error=0, client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:432
>
> Another time:
>
> (top-gdb) bt
> #0  internal_error (file=0x986408 "/home/pedro/gdb/mygit/src/gdb/utils.c", line=1076, fmt=0x986834 "virtual memory exhausted.")
>     at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
> #1  0x000000000074b85d in malloc_failure (size=-79) at /home/pedro/gdb/mygit/src/gdb/utils.c:1076
> #2  0x0000000000783616 in xmalloc (size=18446744073709551537) at /home/pedro/gdb/mygit/src/gdb/common/common-utils.c:43
> #3  0x00000000005256c5 in tui_alloc_source_buffer (win_info=0x28abee0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:621
> #4  0x000000000051ff7b in tui_set_source_content (s=0x2299ae0, line_no=1, noerror=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui-source.c:53
> #5  0x0000000000524871 in tui_update_source_window_as_is (win_info=0x28abee0, gdbarch=0x26a2450, s=0x2299ae0, line_or_addr=..., noerror=1)
>     at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:96
> #6  0x0000000000524824 in tui_update_source_window (win_info=0x28abee0, gdbarch=0x26a2450, s=0x2299ae0, line_or_addr=..., noerror=1)
>     at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:78
> #7  0x000000000052360a in make_visible_with_new_height (win_info=0x28abee0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:1381
> #8  0x000000000052242c in tui_resize_all () at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:723
> #9  0x000000000051d3ac in tui_handle_resize_during_io (original_ch=410) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:769
> #10 0x000000000051d251 in tui_getc (fp=0x3b36db8640 <_IO_2_1_stdin_>) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:693
> #11 0x00000000007aed93 in rl_read_key () at /home/pedro/gdb/mygit/src/readline/input.c:448
> #12 0x0000000000796848 in readline_internal_char () at /home/pedro/gdb/mygit/src/readline/readline.c:517
> #13 0x00000000007af375 in rl_callback_read_char () at /home/pedro/gdb/mygit/src/readline/callback.c:201
> #14 0x000000000063cfb9 in rl_callback_read_char_wrapper (client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:171
> #15 0x000000000063d422 in stdin_event_handler (error=0, client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:432
> #16 0x000000000063bf82 in handle_file_event (data=...) at /home/pedro/gdb/mygit/src/gdb/event-loop.c:762
> #17 0x000000000063b469 in process_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:339
> #18 0x000000000063b530 in gdb_do_one_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:403
> #19 0x000000000063b580 in start_event_loop () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:428
> #20 0x000000000063cfeb in cli_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:186
> #21 0x0000000000633340 in current_interp_command_loop () at /home/pedro/gdb/mygit/src/gdb/interps.c:317
> #22 0x0000000000634509 in captured_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/main.c:321
> #23 0x00000000006301fd in catch_errors (func=0x6344ee <captured_command_loop>, func_args=0x0, errstring=0x942261 "", mask=RETURN_MASK_ALL)
>     at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
> ---Type <return> to continue, or q <return> to quit---
> #24 0x0000000000635a07 in captured_main (data=0x7fffeab9cbb0) at /home/pedro/gdb/mygit/src/gdb/main.c:1149
> #25 0x00000000006301fd in catch_errors (func=0x634906 <captured_main>, func_args=0x7fffeab9cbb0, errstring=0x942261 "", mask=RETURN_MASK_ALL)
>     at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
> #26 0x0000000000635a30 in gdb_main (args=0x7fffeab9cbb0) at /home/pedro/gdb/mygit/src/gdb/main.c:1157
> #27 0x0000000000464e87 in main (argc=3, argv=0x7fffeab9ccb8) at /home/pedro/gdb/mygit/src/gdb/gdb.c:32
>
> Do you see these?

I don't see these because I have the following patch locally
installed: https://sourceware.org/ml/gdb-patches/2014-09/msg00004.html
I planned on resending that patch today or tomorrow.

>
> As a quick try, I added a call to tui_resize_all in tui_enable, and
> it seems to fix these, and almost fix the "stale" size when we
> reenable the TUI, but a subsequent keypress still changes the
> layout, so apparently something more should be done.

Such a change would be convenient, making sure that the TUI screen is
resized correctly from the get-go.  I have a patch for this somewhere,
too...

>
> diff --git c/gdb/tui/tui.c w/gdb/tui/tui.c
> index 00e505d..f3d0fbf 100644
> --- c/gdb/tui/tui.c
> +++ w/gdb/tui/tui.c
> @@ -467,6 +467,7 @@ tui_enable (void)
>          Curses will restore this state when endwin() is called.  */
>       def_shell_mode ();
>       clearok (stdscr, TRUE);
> +     tui_resize_all ();
>     }
>
>    /* Install the TUI specific hooks.  */
>
> Thanks,
> Pedro Alves

Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys

[Patrick Palka]

On Thu, Jan 8, 2015 at 8:22 AM, Pedro Alves <palves@redhat.com> wrote:
> On 01/08/2015 12:32 PM, Patrick Palka wrote:
>> On Thu, Jan 8, 2015 at 6:29 AM, Pedro Alves <palves@redhat.com> wrote:
>>> On 01/08/2015 04:04 AM, Patrick Palka wrote:
>>>> wgetch() sometimes returns KEY_RESIZE when ncurses detects that the
>>>> terminal has been resized.
>>>
>>> I think curses SIGWINCH handler ends up _not_ installed, right?
>>> We install our own, and so does readline.
>>> So how did a resize manage to be detected/processed while inside
>>> wgetch?
>>
>> I'm pretty sure that the SIGWINCH handlers does not get installed.
>> However ncurses may detect a resize event when we exit TUI (exiting
>> ncurses), then resize the terminal, then re-enter TUI (restarting
>> ncurses).  From there a KEY_RESIZE key is added to its internal FIFO.
>> And the next call to wgetch will return this KEY_RESIZE key.
>
> Doesn't that mean then that we're delaying the resize until it's
> "too late"?  IOW, there's a moment where we show the contents with
> the wrong sizes.  And trying that out, I do see that happen: if I
> disable the TUI, resize the terminal, and then reenable the TUI,
> like you say, then until you press _another_ key, the windows
> have the wrong size.  We should have resized them already when we
> re-entered the TUI.

I have just pushed the patch that fixes this issue.  The screen now
gets opportunistically resized when switching from the CLI to the TUI.

>
> And if I try that when I'm already debugging an inferior, stopped at
> a breakpoint, I see nasty things.  If I reduce the size of the
> window, TUI gets stuck in an infinite loop in tui_show_source_line:
>
> (top-gdb) bt
> #0  0x0000003b46008446 in waddch_literal (win=0x28dfd30, ch=32) at ../../ncurses/base/lib_addch.c:253
> #1  0x0000003b4600859c in waddch_nosync (win=0x28dfd30, ch=32) at ../../ncurses/base/lib_addch.c:437
> #2  0x0000003b46008878 in waddch (win=0x28dfd30, ch=32) at ../../ncurses/base/lib_addch.c:543
> #3  0x0000000000524d08 in tui_show_source_line (win_info=0x28dfc90, lineno=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:292
> #4  0x0000000000524d8f in tui_show_source_content (win_info=0x28dfc90) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:305
> #5  0x00000000005220cb in tui_refresh_all_win () at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:633
> #6  0x000000000052611c in tui_enable () at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:486
> #7  0x0000000000525861 in tui_rl_switch_mode (notused1=1, notused2=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:108
> #8  0x0000000000796d6f in _rl_dispatch_subseq (key=1, map=0xd585c0 <emacs_ctlx_keymap>, got_subseq=0) at /home/pedro/gdb/mygit/src/readline/readline.c:774
> #9  0x0000000000796acb in _rl_dispatch_callback (cxt=0x26e6500) at /home/pedro/gdb/mygit/src/readline/readline.c:686
> #10 0x00000000007af20b in rl_callback_read_char () at /home/pedro/gdb/mygit/src/readline/callback.c:170
> #11 0x000000000063cefd in rl_callback_read_char_wrapper (client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:171
> #12 0x000000000063d366 in stdin_event_handler (error=0, client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:432
> #13 0x000000000063bec6 in handle_file_event (data=...) at /home/pedro/gdb/mygit/src/gdb/event-loop.c:762
> ---Type <return> to continue, or q <return> to quit---
> #14 0x000000000063b3ad in process_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:339
> #15 0x000000000063b474 in gdb_do_one_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:403
> #16 0x000000000063b4c4 in start_event_loop () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:428
> #17 0x000000000063cf2f in cli_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:186
> #18 0x0000000000633284 in current_interp_command_loop () at /home/pedro/gdb/mygit/src/gdb/interps.c:317
> #19 0x000000000063444d in captured_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/main.c:321
> #20 0x0000000000630141 in catch_errors (func=0x634432 <captured_command_loop>, func_args=0x0, errstring=0x9422e1 "", mask=RETURN_MASK_ALL)
>     at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
> #21 0x000000000063594b in captured_main (data=0x7fffe05ca000) at /home/pedro/gdb/mygit/src/gdb/main.c:1149
> #22 0x0000000000630141 in catch_errors (func=0x63484a <captured_main>, func_args=0x7fffe05ca000, errstring=0x9422e1 "", mask=RETURN_MASK_ALL)
>     at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
> #23 0x0000000000635974 in gdb_main (args=0x7fffe05ca000) at /home/pedro/gdb/mygit/src/gdb/main.c:1157
> #24 0x0000000000464e57 in main (argc=3, argv=0x7fffe05ca108) at /home/pedro/gdb/mygit/src/gdb/gdb.c:32
>
>
>
> If instead I increase the size of the window, GDB aborts.  E.g.,:
>
> (top-gdb) bt
> #0  internal_error (file=0x986408 "/home/pedro/gdb/mygit/src/gdb/utils.c", line=1076, fmt=0x986834 "virtual memory exhausted.")
>     at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
> #1  0x000000000074b85d in malloc_failure (size=-8) at /home/pedro/gdb/mygit/src/gdb/utils.c:1076
> #2  0x0000000000783616 in xmalloc (size=18446744073709551608) at /home/pedro/gdb/mygit/src/gdb/common/common-utils.c:43
> #3  0x000000000051a13b in tui_alloc_content (num_elements=-1, type=EXEC_INFO_WIN) at /home/pedro/gdb/mygit/src/gdb/tui/tui-data.c:590
> #4  0x00000000005253ee in tui_set_exec_info_content (win_info=0x1a4a6c0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:510
> #5  0x0000000000525672 in tui_update_exec_info (win_info=0x1a4a6c0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:602
> #6  0x00000000005221c8 in tui_refresh_all_win () at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:636
> #7  0x000000000051d3b1 in tui_handle_resize_during_io (original_ch=410) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:770
> #8  0x000000000051d251 in tui_getc (fp=0x3b36db8640 <_IO_2_1_stdin_>) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:693
> #9  0x00000000007aed93 in rl_read_key () at /home/pedro/gdb/mygit/src/readline/input.c:448
> #10 0x0000000000796848 in readline_internal_char () at /home/pedro/gdb/mygit/src/readline/readline.c:517
> #11 0x00000000007af375 in rl_callback_read_char () at /home/pedro/gdb/mygit/src/readline/callback.c:201
> #12 0x000000000063cfb9 in rl_callback_read_char_wrapper (client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:171
> #13 0x000000000063d422 in stdin_event_handler (error=0, client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:432
>
> Another time:
>
> (top-gdb) bt
> #0  internal_error (file=0x986408 "/home/pedro/gdb/mygit/src/gdb/utils.c", line=1076, fmt=0x986834 "virtual memory exhausted.")
>     at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
> #1  0x000000000074b85d in malloc_failure (size=-79) at /home/pedro/gdb/mygit/src/gdb/utils.c:1076
> #2  0x0000000000783616 in xmalloc (size=18446744073709551537) at /home/pedro/gdb/mygit/src/gdb/common/common-utils.c:43
> #3  0x00000000005256c5 in tui_alloc_source_buffer (win_info=0x28abee0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:621
> #4  0x000000000051ff7b in tui_set_source_content (s=0x2299ae0, line_no=1, noerror=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui-source.c:53
> #5  0x0000000000524871 in tui_update_source_window_as_is (win_info=0x28abee0, gdbarch=0x26a2450, s=0x2299ae0, line_or_addr=..., noerror=1)
>     at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:96
> #6  0x0000000000524824 in tui_update_source_window (win_info=0x28abee0, gdbarch=0x26a2450, s=0x2299ae0, line_or_addr=..., noerror=1)
>     at /home/pedro/gdb/mygit/src/gdb/tui/tui-winsource.c:78
> #7  0x000000000052360a in make_visible_with_new_height (win_info=0x28abee0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:1381
> #8  0x000000000052242c in tui_resize_all () at /home/pedro/gdb/mygit/src/gdb/tui/tui-win.c:723
> #9  0x000000000051d3ac in tui_handle_resize_during_io (original_ch=410) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:769
> #10 0x000000000051d251 in tui_getc (fp=0x3b36db8640 <_IO_2_1_stdin_>) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:693
> #11 0x00000000007aed93 in rl_read_key () at /home/pedro/gdb/mygit/src/readline/input.c:448
> #12 0x0000000000796848 in readline_internal_char () at /home/pedro/gdb/mygit/src/readline/readline.c:517
> #13 0x00000000007af375 in rl_callback_read_char () at /home/pedro/gdb/mygit/src/readline/callback.c:201
> #14 0x000000000063cfb9 in rl_callback_read_char_wrapper (client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:171
> #15 0x000000000063d422 in stdin_event_handler (error=0, client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:432
> #16 0x000000000063bf82 in handle_file_event (data=...) at /home/pedro/gdb/mygit/src/gdb/event-loop.c:762
> #17 0x000000000063b469 in process_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:339
> #18 0x000000000063b530 in gdb_do_one_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:403
> #19 0x000000000063b580 in start_event_loop () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:428
> #20 0x000000000063cfeb in cli_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:186
> #21 0x0000000000633340 in current_interp_command_loop () at /home/pedro/gdb/mygit/src/gdb/interps.c:317
> #22 0x0000000000634509 in captured_command_loop (data=0x0) at /home/pedro/gdb/mygit/src/gdb/main.c:321
> #23 0x00000000006301fd in catch_errors (func=0x6344ee <captured_command_loop>, func_args=0x0, errstring=0x942261 "", mask=RETURN_MASK_ALL)
>     at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
> ---Type <return> to continue, or q <return> to quit---
> #24 0x0000000000635a07 in captured_main (data=0x7fffeab9cbb0) at /home/pedro/gdb/mygit/src/gdb/main.c:1149
> #25 0x00000000006301fd in catch_errors (func=0x634906 <captured_main>, func_args=0x7fffeab9cbb0, errstring=0x942261 "", mask=RETURN_MASK_ALL)
>     at /home/pedro/gdb/mygit/src/gdb/exceptions.c:237
> #26 0x0000000000635a30 in gdb_main (args=0x7fffeab9cbb0) at /home/pedro/gdb/mygit/src/gdb/main.c:1157
> #27 0x0000000000464e87 in main (argc=3, argv=0x7fffeab9ccb8) at /home/pedro/gdb/mygit/src/gdb/gdb.c:32
>
> Do you see these?
>
> As a quick try, I added a call to tui_resize_all in tui_enable, and
> it seems to fix these, and almost fix the "stale" size when we
> reenable the TUI, but a subsequent keypress still changes the
> layout, so apparently something more should be done.
>
> diff --git c/gdb/tui/tui.c w/gdb/tui/tui.c
> index 00e505d..f3d0fbf 100644
> --- c/gdb/tui/tui.c
> +++ w/gdb/tui/tui.c
> @@ -467,6 +467,7 @@ tui_enable (void)
>          Curses will restore this state when endwin() is called.  */
>       def_shell_mode ();
>       clearok (stdscr, TRUE);
> +     tui_resize_all ();
>     }
>
>    /* Install the TUI specific hooks.  */

The pushed fix was essentially that, but it also sets tui_win_resized
to FALSE so that a subsequent keypress does not change the layout.  Is
this patch now OK?  (You have already approved patch #1 and #3 in this
series.)

>
> Thanks,
> Pedro Alves

Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys

[Pedro Alves]

On 02/11/2015 12:25 AM, Patrick Palka wrote:
> On Thu, Jan 8, 2015 at 8:22 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 01/08/2015 12:32 PM, Patrick Palka wrote:
>>> On Thu, Jan 8, 2015 at 6:29 AM, Pedro Alves <palves@redhat.com> wrote:
>>>> On 01/08/2015 04:04 AM, Patrick Palka wrote:
>>>>> wgetch() sometimes returns KEY_RESIZE when ncurses detects that the
>>>>> terminal has been resized.
>>>>
>>>> I think curses SIGWINCH handler ends up _not_ installed, right?
>>>> We install our own, and so does readline.
>>>> So how did a resize manage to be detected/processed while inside
>>>> wgetch?
>>>
>>> I'm pretty sure that the SIGWINCH handlers does not get installed.
>>> However ncurses may detect a resize event when we exit TUI (exiting
>>> ncurses), then resize the terminal, then re-enter TUI (restarting
>>> ncurses).  From there a KEY_RESIZE key is added to its internal FIFO.
>>> And the next call to wgetch will return this KEY_RESIZE key.
>>
>> Doesn't that mean then that we're delaying the resize until it's
>> "too late"?  IOW, there's a moment where we show the contents with
>> the wrong sizes.  And trying that out, I do see that happen: if I
>> disable the TUI, resize the terminal, and then reenable the TUI,
>> like you say, then until you press _another_ key, the windows
>> have the wrong size.  We should have resized them already when we
>> re-entered the TUI.
> 
> I have just pushed the patch that fixes this issue.  The screen now
> gets opportunistically resized when switching from the CLI to the TUI.

...

> The pushed fix was essentially that, but it also sets tui_win_resized
> to FALSE so that a subsequent keypress does not change the layout.  Is
> this patch now OK?  (You have already approved patch #1 and #3 in this
> series.)

Sorry, I'm still not convinced.  :-/  ISTM that KEY_RESIZE just
hides latent issues/design mistakes.

If we have a SIGWINCH handler, and we detect the need to resize when
we're re-enabling the TUI, and explicitly resize all windows in that
case, why would ncurses still detect a resize and put a KEY_RESIZE in
the wgetch queue?

ISTM that we're just resizing too late still.  Indeed, the patch below,
which makes us resize earlier, fixes the issue for me too.  Are you
aware of any other use case that might cause KEY_RESIZE?

(Note: Whatever the order of calls in tui_enable, we'll potentially be
showing stale contents momentarily and then overwriting with correct ones.
We get that today too.  IMO, today's even worse, as it can show windows with
the wrong size for a moment, and that might be more visible flicker than showing
the wrong contents with the correct border sizes already.  ISTM the fix for
that would be to decouple "update windows' contents" from actually
wrefresh/display'ing windows.  That looks like much more work than worth
it though.  I can only see this if I step through the code within tui_enable.
In a normal not-being-debugged run, I can't notice any flicker.)

---
From: Pedro Alves <palves@redhat.com>
Subject: [PATCH] TUI: resize windows to new terminal size before displaying
 them

If the user:

   #1 - disables the TUI
   #2 - resizes the terminal
   #3 - and then re-enables the TUI

the next wgetch() returns KEY_RESIZE.  This indicates to the ncurses
client that that ncurses detected that the terminal has been resized.
We don't handle KEY_RESIZE anywhere, so it gets passed on to readline
which interprets it as a multibyte character, and then the end result
is that the first key press after enabling the TUI is misinterpreted.

We shouldn't really need to handle KEY_RESIZE (and not all ncurses
implementations have that).  We have our own SIGWINCH handler, and,
when we re-enable the TUI, we explicitly detect terminal resizes and
resize all windows.  The reason ncurses currently does detects a
resize is that something within tui_enable forces a refresh/display of
some window before we get to do the actual resizing.  Setting a break
on ncurses' 'resizeterm' function helps find the culprit(s):

 (top-gdb) bt
 #0  resizeterm (ToLines=28, ToCols=114) at ../../ncurses/base/resizeterm.c:462
 #1  0x0000003b42812f3f in _nc_update_screensize (sp=0x2674730) at ../../ncurses/tinfo/lib_setup.c:443
 #2  0x0000003b0821cbe0 in doupdate () at ../../ncurses/tty/tty_update.c:726
 #3  0x0000003b08215539 in wrefresh (win=0x2a7bc00) at ../../ncurses/base/lib_refresh.c:65
 #4  0x00000000005257cb in tui_refresh_win (win_info=0xd73d60 <_locator>) at /home/pedro/gdb/mygit/src/gdb/tui/tui-wingeneral.c:60
 #5  0x000000000052265b in tui_show_locator_content () at /home/pedro/gdb/mygit/src/gdb/tui/tui-stack.c:269
 #6  0x00000000005273a6 in tui_set_key_mode (mode=TUI_COMMAND_MODE) at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:321
 #7  0x00000000005278c7 in tui_enable () at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:494
 #8  0x0000000000527011 in tui_rl_switch_mode (notused1=1, notused2=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:108

That is, tui_enable calls tui_set_key_mode before we've resized all
windows, and that refreshes a window as side effect.

And if we're already debugging something (there's a frame), then we'll
instead show a window from within tui_show_frame_info:

 (top-gdb) bt
 #0  resizeterm (ToLines=28, ToCols=114) at ../../ncurses/base/resizeterm.c:462
 #1  0x0000003b42812f3f in _nc_update_screensize (sp=0x202e6c0) at ../../ncurses/tinfo/lib_setup.c:443
 #2  0x0000003b0821cbe0 in doupdate () at ../../ncurses/tty/tty_update.c:726
 #3  0x0000003b08215539 in wrefresh (win=0x2042890) at ../../ncurses/base/lib_refresh.c:65
 #4  0x00000000005257cb in tui_refresh_win (win_info=0xd73d60 <_locator>) at /home/pedro/gdb/mygit/src/gdb/tui/tui-wingeneral.c:60
 #5  0x000000000052265b in tui_show_locator_content () at /home/pedro/gdb/mygit/src/gdb/tui/tui-stack.c:269
 #6  0x0000000000522931 in tui_show_frame_info (fi=0x16b9cc0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-stack.c:364
 #7  0x00000000005278ba in tui_enable () at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:491
 #8  0x0000000000527011 in tui_rl_switch_mode (notused1=1, notused2=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:108

The fix is to resize windows earlier.

gdb/ChangeLog:
2015-02-16 Pedro Alves <palves@redhat.com>

	* tui/tui.c (tui_enable): Resize windows before anything
	might show a window.
---
 gdb/tui/tui.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 834e682..0397ee9 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -487,18 +487,22 @@ tui_enable (void)
   tui_setup_io (1);
 
   tui_active = 1;
-  if (deprecated_safe_get_selected_frame ())
-     tui_show_frame_info (deprecated_safe_get_selected_frame ());
 
-  /* Restore TUI keymap.  */
-  tui_set_key_mode (tui_current_key_mode);
-
-  /* Resize and refresh the screen.  */
+  /* Resize windows before anything might display/refresh a
+     window.  */
   if (tui_win_resized ())
     {
       tui_resize_all ();
       tui_set_win_resized_to (FALSE);
     }
+
+  if (deprecated_safe_get_selected_frame ())
+    tui_show_frame_info (deprecated_safe_get_selected_frame ());
+
+  /* Restore TUI keymap.  */
+  tui_set_key_mode (tui_current_key_mode);
+
+  /* Refresh the screen.  */
   tui_refresh_all_win ();
 
   /* Update gdb's knowledge of its terminal.  */
-- 
1.9.3

Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys

[Patrick Palka]

On Mon, Feb 16, 2015 at 5:42 PM, Pedro Alves <palves@redhat.com> wrote:
> On 02/11/2015 12:25 AM, Patrick Palka wrote:
>> On Thu, Jan 8, 2015 at 8:22 AM, Pedro Alves <palves@redhat.com> wrote:
>>> On 01/08/2015 12:32 PM, Patrick Palka wrote:
>>>> On Thu, Jan 8, 2015 at 6:29 AM, Pedro Alves <palves@redhat.com> wrote:
>>>>> On 01/08/2015 04:04 AM, Patrick Palka wrote:
>>>>>> wgetch() sometimes returns KEY_RESIZE when ncurses detects that the
>>>>>> terminal has been resized.
>>>>>
>>>>> I think curses SIGWINCH handler ends up _not_ installed, right?
>>>>> We install our own, and so does readline.
>>>>> So how did a resize manage to be detected/processed while inside
>>>>> wgetch?
>>>>
>>>> I'm pretty sure that the SIGWINCH handlers does not get installed.
>>>> However ncurses may detect a resize event when we exit TUI (exiting
>>>> ncurses), then resize the terminal, then re-enter TUI (restarting
>>>> ncurses).  From there a KEY_RESIZE key is added to its internal FIFO.
>>>> And the next call to wgetch will return this KEY_RESIZE key.
>>>
>>> Doesn't that mean then that we're delaying the resize until it's
>>> "too late"?  IOW, there's a moment where we show the contents with
>>> the wrong sizes.  And trying that out, I do see that happen: if I
>>> disable the TUI, resize the terminal, and then reenable the TUI,
>>> like you say, then until you press _another_ key, the windows
>>> have the wrong size.  We should have resized them already when we
>>> re-entered the TUI.
>>
>> I have just pushed the patch that fixes this issue.  The screen now
>> gets opportunistically resized when switching from the CLI to the TUI.
>
> ...
>
>> The pushed fix was essentially that, but it also sets tui_win_resized
>> to FALSE so that a subsequent keypress does not change the layout.  Is
>> this patch now OK?  (You have already approved patch #1 and #3 in this
>> series.)
>
> Sorry, I'm still not convinced.  :-/  ISTM that KEY_RESIZE just
> hides latent issues/design mistakes.
>
> If we have a SIGWINCH handler, and we detect the need to resize when
> we're re-enabling the TUI, and explicitly resize all windows in that
> case, why would ncurses still detect a resize and put a KEY_RESIZE in
> the wgetch queue?

Good point...  When leaving TUI mode, we disable ncurses via endwin().
When re-entering TUI mode, we are re-enabling ncurses through the
first call to refresh().  If we manage to sync ncurses' knowledge of
the terminal dimensions (via the call to resize_terminall() in
tui_resize_all() I believe) before re-enabling ncurses then a
KEY_RESIZE should not get placed into the queue.

>
> ISTM that we're just resizing too late still.  Indeed, the patch below,
> which makes us resize earlier, fixes the issue for me too.  Are you
> aware of any other use case that might cause KEY_RESIZE?

Nice, it fixes the issue for me too.  No more KEY_RESIZE keys.  And I
am not aware of another way to trigger KEY_RESIZE keys.

>
> (Note: Whatever the order of calls in tui_enable, we'll potentially be
> showing stale contents momentarily and then overwriting with correct ones.
> We get that today too.  IMO, today's even worse, as it can show windows with
> the wrong size for a moment, and that might be more visible flicker than showing
> the wrong contents with the correct border sizes already.  ISTM the fix for
> that would be to decouple "update windows' contents" from actually
> wrefresh/display'ing windows.  That looks like much more work than worth
> it though.  I can only see this if I step through the code within tui_enable.
> In a normal not-being-debugged run, I can't notice any flicker.)

I have never noticed such flickering.  But it is does not surprise me
that TUI has the potential to flicker like hell, see
https://sourceware.org/bugzilla/show_bug.cgi?id=13378

The patch makes a lot of sense to me... I appreciate your taking such
an in-depth look at such a tedious issue!  Is it OK to commit patches
1 and 3 of this series once you commit the below patch?

(As a side note it boggles my mind that [vertically] resizing the
terminal while inside TUI is horribly broken, yet indirectly resizing
TUI by temporarily exiting TUI, resizing within the CLI then
re-entering TUI works just fine.  Both of these methods ought to be
running the same resizing logic!  Something tells me most of
tui_resize_all() is unnecessary and broken.)

>
> ---
> From: Pedro Alves <palves@redhat.com>
> Subject: [PATCH] TUI: resize windows to new terminal size before displaying
>  them
>
> If the user:
>
>    #1 - disables the TUI
>    #2 - resizes the terminal
>    #3 - and then re-enables the TUI
>
> the next wgetch() returns KEY_RESIZE.  This indicates to the ncurses
> client that that ncurses detected that the terminal has been resized.
> We don't handle KEY_RESIZE anywhere, so it gets passed on to readline
> which interprets it as a multibyte character, and then the end result
> is that the first key press after enabling the TUI is misinterpreted.
>
> We shouldn't really need to handle KEY_RESIZE (and not all ncurses
> implementations have that).  We have our own SIGWINCH handler, and,
> when we re-enable the TUI, we explicitly detect terminal resizes and
> resize all windows.  The reason ncurses currently does detects a
> resize is that something within tui_enable forces a refresh/display of
> some window before we get to do the actual resizing.  Setting a break
> on ncurses' 'resizeterm' function helps find the culprit(s):
>
>  (top-gdb) bt
>  #0  resizeterm (ToLines=28, ToCols=114) at ../../ncurses/base/resizeterm.c:462
>  #1  0x0000003b42812f3f in _nc_update_screensize (sp=0x2674730) at ../../ncurses/tinfo/lib_setup.c:443
>  #2  0x0000003b0821cbe0 in doupdate () at ../../ncurses/tty/tty_update.c:726
>  #3  0x0000003b08215539 in wrefresh (win=0x2a7bc00) at ../../ncurses/base/lib_refresh.c:65
>  #4  0x00000000005257cb in tui_refresh_win (win_info=0xd73d60 <_locator>) at /home/pedro/gdb/mygit/src/gdb/tui/tui-wingeneral.c:60
>  #5  0x000000000052265b in tui_show_locator_content () at /home/pedro/gdb/mygit/src/gdb/tui/tui-stack.c:269
>  #6  0x00000000005273a6 in tui_set_key_mode (mode=TUI_COMMAND_MODE) at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:321
>  #7  0x00000000005278c7 in tui_enable () at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:494
>  #8  0x0000000000527011 in tui_rl_switch_mode (notused1=1, notused2=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:108
>
> That is, tui_enable calls tui_set_key_mode before we've resized all
> windows, and that refreshes a window as side effect.
>
> And if we're already debugging something (there's a frame), then we'll
> instead show a window from within tui_show_frame_info:
>
>  (top-gdb) bt
>  #0  resizeterm (ToLines=28, ToCols=114) at ../../ncurses/base/resizeterm.c:462
>  #1  0x0000003b42812f3f in _nc_update_screensize (sp=0x202e6c0) at ../../ncurses/tinfo/lib_setup.c:443
>  #2  0x0000003b0821cbe0 in doupdate () at ../../ncurses/tty/tty_update.c:726
>  #3  0x0000003b08215539 in wrefresh (win=0x2042890) at ../../ncurses/base/lib_refresh.c:65
>  #4  0x00000000005257cb in tui_refresh_win (win_info=0xd73d60 <_locator>) at /home/pedro/gdb/mygit/src/gdb/tui/tui-wingeneral.c:60
>  #5  0x000000000052265b in tui_show_locator_content () at /home/pedro/gdb/mygit/src/gdb/tui/tui-stack.c:269
>  #6  0x0000000000522931 in tui_show_frame_info (fi=0x16b9cc0) at /home/pedro/gdb/mygit/src/gdb/tui/tui-stack.c:364
>  #7  0x00000000005278ba in tui_enable () at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:491
>  #8  0x0000000000527011 in tui_rl_switch_mode (notused1=1, notused2=1) at /home/pedro/gdb/mygit/src/gdb/tui/tui.c:108
>
> The fix is to resize windows earlier.
>
> gdb/ChangeLog:
> 2015-02-16 Pedro Alves <palves@redhat.com>
>
>         * tui/tui.c (tui_enable): Resize windows before anything
>         might show a window.
> ---
>  gdb/tui/tui.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
> index 834e682..0397ee9 100644
> --- a/gdb/tui/tui.c
> +++ b/gdb/tui/tui.c
> @@ -487,18 +487,22 @@ tui_enable (void)
>    tui_setup_io (1);
>
>    tui_active = 1;
> -  if (deprecated_safe_get_selected_frame ())
> -     tui_show_frame_info (deprecated_safe_get_selected_frame ());
>
> -  /* Restore TUI keymap.  */
> -  tui_set_key_mode (tui_current_key_mode);
> -
> -  /* Resize and refresh the screen.  */
> +  /* Resize windows before anything might display/refresh a
> +     window.  */
>    if (tui_win_resized ())
>      {
>        tui_resize_all ();
>        tui_set_win_resized_to (FALSE);
>      }
> +
> +  if (deprecated_safe_get_selected_frame ())
> +    tui_show_frame_info (deprecated_safe_get_selected_frame ());
> +
> +  /* Restore TUI keymap.  */
> +  tui_set_key_mode (tui_current_key_mode);
> +
> +  /* Refresh the screen.  */
>    tui_refresh_all_win ();
>
>    /* Update gdb's knowledge of its terminal.  */
> --
> 1.9.3
>

Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys

[Pedro Alves]

On 02/17/2015 12:52 AM, Patrick Palka wrote:
> On Mon, Feb 16, 2015 at 5:42 PM, Pedro Alves <palves@redhat.com> wrote:

> Good point...  When leaving TUI mode, we disable ncurses via endwin().
> When re-entering TUI mode, we are re-enabling ncurses through the
> first call to refresh().  If we manage to sync ncurses' knowledge of
> the terminal dimensions (via the call to resize_terminall() in
> tui_resize_all() I believe) before re-enabling ncurses then a
> KEY_RESIZE should not get placed into the queue.

Exactly.

> 
>>
>> ISTM that we're just resizing too late still.  Indeed, the patch below,
>> which makes us resize earlier, fixes the issue for me too.  Are you
>> aware of any other use case that might cause KEY_RESIZE?
> 
> Nice, it fixes the issue for me too.  No more KEY_RESIZE keys.  And I
> am not aware of another way to trigger KEY_RESIZE keys.

Alright, great.

>> (Note: Whatever the order of calls in tui_enable, we'll potentially be
>> showing stale contents momentarily and then overwriting with correct ones.
>> We get that today too.  IMO, today's even worse, as it can show windows with
>> the wrong size for a moment, and that might be more visible flicker than showing
>> the wrong contents with the correct border sizes already.  ISTM the fix for
>> that would be to decouple "update windows' contents" from actually
>> wrefresh/display'ing windows.  That looks like much more work than worth
>> it though.  I can only see this if I step through the code within tui_enable.
>> In a normal not-being-debugged run, I can't notice any flicker.)
> 
> I have never noticed such flickering.  But it is does not surprise me
> that TUI has the potential to flicker like hell, see
> https://sourceware.org/bugzilla/show_bug.cgi?id=13378

Yeah...

> The patch makes a lot of sense to me... I appreciate your taking such
> an in-depth look at such a tedious issue!

:-)  These discussions have resulted in several fixes (from you), so
I think it's been well worth it, IMO.  Thank _you_ for poking at
the TUI.

> Is it OK to commit patches
> 1 and 3 of this series once you commit the below patch?

Yes please.  I've now pushed mine in.

> 
> (As a side note it boggles my mind that [vertically] resizing the
> terminal while inside TUI is horribly broken, yet indirectly resizing
> TUI by temporarily exiting TUI, resizing within the CLI then
> re-entering TUI works just fine.  Both of these methods ought to be
> running the same resizing logic!  Something tells me most of
> tui_resize_all() is unnecessary and broken.)

I think that that's missing is integrating SIGWINCH handling with the
event loop.  That is, nothing is waking up the event loop to react to
SIGWINCH and do the resize, so the resize ends up happening on
next IO (next key press).  Something along these lines:

In the TUI setup code where we install the SIGWINCH signal
handler create a new event loop source for the SIGWINCH signal:

  sigwinch_token =
    create_async_signal_handler (async_sigwinch_handler, NULL);

Have the real SIGWINCH handler mark that async event source:

static void
handle_sigwinch (int sig)
{
  mark_async_signal_handler (sigwinch_token);
}

And then when that event source's callback is called
by the event loop, resize the TUI:

static void
async_sigwinch_handler (gdb_client_data arg)
{
  /* Trigger TUI resize here.  */
}

Would you like to try that?

Thanks,
Pedro Alves

Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys

[Patrick Palka]

On Tue, Feb 17, 2015 at 5:23 AM, Pedro Alves <palves@redhat.com> wrote:
> On 02/17/2015 12:52 AM, Patrick Palka wrote:
>> On Mon, Feb 16, 2015 at 5:42 PM, Pedro Alves <palves@redhat.com> wrote:
>
>> Good point...  When leaving TUI mode, we disable ncurses via endwin().
>> When re-entering TUI mode, we are re-enabling ncurses through the
>> first call to refresh().  If we manage to sync ncurses' knowledge of
>> the terminal dimensions (via the call to resize_terminall() in
>> tui_resize_all() I believe) before re-enabling ncurses then a
>> KEY_RESIZE should not get placed into the queue.
>
> Exactly.
>
>>
>>>
>>> ISTM that we're just resizing too late still.  Indeed, the patch below,
>>> which makes us resize earlier, fixes the issue for me too.  Are you
>>> aware of any other use case that might cause KEY_RESIZE?
>>
>> Nice, it fixes the issue for me too.  No more KEY_RESIZE keys.  And I
>> am not aware of another way to trigger KEY_RESIZE keys.
>
> Alright, great.
>
>>> (Note: Whatever the order of calls in tui_enable, we'll potentially be
>>> showing stale contents momentarily and then overwriting with correct ones.
>>> We get that today too.  IMO, today's even worse, as it can show windows with
>>> the wrong size for a moment, and that might be more visible flicker than showing
>>> the wrong contents with the correct border sizes already.  ISTM the fix for
>>> that would be to decouple "update windows' contents" from actually
>>> wrefresh/display'ing windows.  That looks like much more work than worth
>>> it though.  I can only see this if I step through the code within tui_enable.
>>> In a normal not-being-debugged run, I can't notice any flicker.)
>>
>> I have never noticed such flickering.  But it is does not surprise me
>> that TUI has the potential to flicker like hell, see
>> https://sourceware.org/bugzilla/show_bug.cgi?id=13378
>
> Yeah...
>
>> The patch makes a lot of sense to me... I appreciate your taking such
>> an in-depth look at such a tedious issue!
>
> :-)  These discussions have resulted in several fixes (from you), so
> I think it's been well worth it, IMO.  Thank _you_ for poking at
> the TUI.
>
>> Is it OK to commit patches
>> 1 and 3 of this series once you commit the below patch?
>
> Yes please.  I've now pushed mine in.
>
>>
>> (As a side note it boggles my mind that [vertically] resizing the
>> terminal while inside TUI is horribly broken, yet indirectly resizing
>> TUI by temporarily exiting TUI, resizing within the CLI then
>> re-entering TUI works just fine.  Both of these methods ought to be
>> running the same resizing logic!  Something tells me most of
>> tui_resize_all() is unnecessary and broken.)
>
> I think that that's missing is integrating SIGWINCH handling with the
> event loop.  That is, nothing is waking up the event loop to react to
> SIGWINCH and do the resize, so the resize ends up happening on
> next IO (next key press).  Something along these lines:

That would be nice.

>
> In the TUI setup code where we install the SIGWINCH signal
> handler create a new event loop source for the SIGWINCH signal:
>
>   sigwinch_token =
>     create_async_signal_handler (async_sigwinch_handler, NULL);
>
> Have the real SIGWINCH handler mark that async event source:
>
> static void
> handle_sigwinch (int sig)
> {
>   mark_async_signal_handler (sigwinch_token);
> }
>
> And then when that event source's callback is called
> by the event loop, resize the TUI:
>
> static void
> async_sigwinch_handler (gdb_client_data arg)
> {
>   /* Trigger TUI resize here.  */
> }
>
> Would you like to try that?

Didn't know that adding another event handler is so easy.  I'll try doing that.

>
> Thanks,
> Pedro Alves
>

Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys

[Patrick Palka]

On Tue, Feb 17, 2015 at 5:23 AM, Pedro Alves <palves@redhat.com> wrote:
> On 02/17/2015 12:52 AM, Patrick Palka wrote:
>> On Mon, Feb 16, 2015 at 5:42 PM, Pedro Alves <palves@redhat.com> wrote:
>
>> Good point...  When leaving TUI mode, we disable ncurses via endwin().
>> When re-entering TUI mode, we are re-enabling ncurses through the
>> first call to refresh().  If we manage to sync ncurses' knowledge of
>> the terminal dimensions (via the call to resize_terminall() in
>> tui_resize_all() I believe) before re-enabling ncurses then a
>> KEY_RESIZE should not get placed into the queue.
>
> Exactly.
>
>>
>>>
>>> ISTM that we're just resizing too late still.  Indeed, the patch below,
>>> which makes us resize earlier, fixes the issue for me too.  Are you
>>> aware of any other use case that might cause KEY_RESIZE?
>>
>> Nice, it fixes the issue for me too.  No more KEY_RESIZE keys.  And I
>> am not aware of another way to trigger KEY_RESIZE keys.
>
> Alright, great.
>
>>> (Note: Whatever the order of calls in tui_enable, we'll potentially be
>>> showing stale contents momentarily and then overwriting with correct ones.
>>> We get that today too.  IMO, today's even worse, as it can show windows with
>>> the wrong size for a moment, and that might be more visible flicker than showing
>>> the wrong contents with the correct border sizes already.  ISTM the fix for
>>> that would be to decouple "update windows' contents" from actually
>>> wrefresh/display'ing windows.  That looks like much more work than worth
>>> it though.  I can only see this if I step through the code within tui_enable.
>>> In a normal not-being-debugged run, I can't notice any flicker.)
>>
>> I have never noticed such flickering.  But it is does not surprise me
>> that TUI has the potential to flicker like hell, see
>> https://sourceware.org/bugzilla/show_bug.cgi?id=13378
>
> Yeah...
>
>> The patch makes a lot of sense to me... I appreciate your taking such
>> an in-depth look at such a tedious issue!
>
> :-)  These discussions have resulted in several fixes (from you), so
> I think it's been well worth it, IMO.  Thank _you_ for poking at
> the TUI.
>
>> Is it OK to commit patches
>> 1 and 3 of this series once you commit the below patch?
>
> Yes please.  I've now pushed mine in.

Actually, I will not commit patch #3 because it no longer applies
cleanly after the CLI/TUI tab completion consolidation and
incorporating sigwinch handling will allow for the complete
eradication of tui_handle_resize_during_io().

>
>>
>> (As a side note it boggles my mind that [vertically] resizing the
>> terminal while inside TUI is horribly broken, yet indirectly resizing
>> TUI by temporarily exiting TUI, resizing within the CLI then
>> re-entering TUI works just fine.  Both of these methods ought to be
>> running the same resizing logic!  Something tells me most of
>> tui_resize_all() is unnecessary and broken.)
>
> I think that that's missing is integrating SIGWINCH handling with the
> event loop.  That is, nothing is waking up the event loop to react to
> SIGWINCH and do the resize, so the resize ends up happening on
> next IO (next key press).  Something along these lines:
>
> In the TUI setup code where we install the SIGWINCH signal
> handler create a new event loop source for the SIGWINCH signal:
>
>   sigwinch_token =
>     create_async_signal_handler (async_sigwinch_handler, NULL);
>
> Have the real SIGWINCH handler mark that async event source:
>
> static void
> handle_sigwinch (int sig)
> {
>   mark_async_signal_handler (sigwinch_token);
> }
>
> And then when that event source's callback is called
> by the event loop, resize the TUI:
>
> static void
> async_sigwinch_handler (gdb_client_data arg)
> {
>   /* Trigger TUI resize here.  */
> }
>
> Would you like to try that?
>
> Thanks,
> Pedro Alves
>

[PATCH 3/3] Don't flush the prompt when resizing the terminal within TUI

[Patrick Palka]

This patch removes the ancient code that is responsible for forcing the
prompt to get flushed and executed when tui_getc() detects that the
terminal has been resized.  This behavior is unintuitive and seemingly
unnecessary.  I tried figuring out why tui_getc() behaves this way, but
git-blame does not reveal anything informative about this code.  It
probably has something to do with avoiding to print KEY_RESIZE keys, but
the previous patch prevents that from happening anymore.

gdb/ChangeLog:

	* tui/tui-io.c (tui_handle_resize_during_io): Remove parameter.
	Change return type to void.  Don't call dont_repeat.
	(tui_getc): Adjust.
---
 gdb/tui/tui-io.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index b0e6c75..f072bdb 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -135,7 +135,7 @@ static int tui_readline_pipe[2];
    This may be the main gdb prompt or a secondary prompt.  */
 static char *tui_rl_saved_prompt;
 
-static unsigned int tui_handle_resize_during_io (unsigned int);
+static void tui_handle_resize_during_io (void);
 
 static void
 tui_putc (char c)
@@ -642,7 +642,8 @@ tui_getc (FILE *fp)
 #endif
 
   ch = wgetch (w);
-  ch = tui_handle_resize_during_io (ch);
+
+  tui_handle_resize_during_io ();
 
   /* The \n must be echoed because it will not be printed by
      readline.  */
@@ -709,19 +710,15 @@ tui_getc (FILE *fp)
 }
 
 
-/* Cleanup when a resize has occured.
-   Returns the character that must be processed.  */
-static unsigned int
-tui_handle_resize_during_io (unsigned int original_ch)
+/* Cleanup when a resize has occured.  */
+
+static void
+tui_handle_resize_during_io (void)
 {
   if (tui_win_resized ())
     {
       tui_resize_all ();
       tui_refresh_all_win ();
-      dont_repeat ();
       tui_set_win_resized_to (FALSE);
-      return '\n';
     }
-  else
-    return original_ch;
 }
-- 
2.2.1.212.gc5b9256

Re: [PATCH 3/3] Don't flush the prompt when resizing the terminal within TUI

[Pedro Alves]

On 01/08/2015 04:04 AM, Patrick Palka wrote:
> This patch removes the ancient code that is responsible for forcing the
> prompt to get flushed and executed when tui_getc() detects that the
> terminal has been resized.  This behavior is unintuitive and seemingly
> unnecessary.  I tried figuring out why tui_getc() behaves this way, but
> git-blame does not reveal anything informative about this code.  It
> probably has something to do with avoiding to print KEY_RESIZE keys, but
> the previous patch prevents that from happening anymore.

I don't know why it was found necessary either.  I agree it's not intuitive.

Looks good to me.

Thanks,
Pedro Alves

Re: [PATCH 1/3] Remove superfluous function key_is_command_char()

[Pedro Alves]

On 01/08/2015 04:04 AM, Patrick Palka wrote:
> The function key_is_command_char() is simply a predicate that determines
> whether the function tui_dispatch_ctrl_char() will do anything useful.
> Since tui_dispatch_ctrl_char() performs the same checks as
> key_is_command_char() it is unnecessary to keep key_is_command_char()
> around.  This patch removes this useless function and instead
> unconditionally calls tui_dispatch_ctrl_char() inside its only caller,
> tui_getc().
> 
> gdb/ChangeLog:
> 
> 	* tui/tui-io.c (tui_getc): Don't call key_is_command_char.
> 	(key_is_command_char): Delete.

OK.

Thanks,
Pedro Alves

Re: [PATCH 1/3] Remove superfluous function key_is_command_char()

[Eli Zaretskii]

> From: Patrick Palka <patrick@parcs.ath.cx>
> Cc: Patrick Palka <patrick@parcs.ath.cx>
> Date: Wed,  7 Jan 2015 23:04:43 -0500
> 
> The function key_is_command_char() is simply a predicate that determines
> whether the function tui_dispatch_ctrl_char() will do anything useful.
> Since tui_dispatch_ctrl_char() performs the same checks as
> key_is_command_char() it is unnecessary to keep key_is_command_char()
> around.  This patch removes this useless function and instead
> unconditionally calls tui_dispatch_ctrl_char() inside its only caller,
> tui_getc().

But tui_dispatch_ctrl_char punts when the current window is the
command window, doesn't it?  This means we are losing the possibility
to handle command keys in the command window.

Thanks.

Re: [PATCH 1/3] Remove superfluous function key_is_command_char()

[Patrick Palka]

On Thu, Jan 8, 2015 at 8:34 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Patrick Palka <patrick@parcs.ath.cx>
>> Cc: Patrick Palka <patrick@parcs.ath.cx>
>> Date: Wed,  7 Jan 2015 23:04:43 -0500
>>
>> The function key_is_command_char() is simply a predicate that determines
>> whether the function tui_dispatch_ctrl_char() will do anything useful.
>> Since tui_dispatch_ctrl_char() performs the same checks as
>> key_is_command_char() it is unnecessary to keep key_is_command_char()
>> around.  This patch removes this useless function and instead
>> unconditionally calls tui_dispatch_ctrl_char() inside its only caller,
>> tui_getc().
>
> But tui_dispatch_ctrl_char punts when the current window is the
> command window, doesn't it?  This means we are losing the possibility
> to handle command keys in the command window.

I don't see how that follows.  If the current window is the command
window then dispatch_ctrl_char() returns the input character
unmodified.  So the behavior after the patch is the same as if the
call to dispatch_ctrl_char() was guarded by the key_is_command_char()
predicate, before the patch.

>
> Thanks.

Re: [PATCH 1/3] Remove superfluous function key_is_command_char()

[Eli Zaretskii]

> From: Patrick Palka <patrick@parcs.ath.cx>
> Date: Thu, 8 Jan 2015 08:47:44 -0500
> Cc: gdb-patches@sourceware.org
> 
> > But tui_dispatch_ctrl_char punts when the current window is the
> > command window, doesn't it?  This means we are losing the possibility
> > to handle command keys in the command window.
> 
> I don't see how that follows.  If the current window is the command
> window then dispatch_ctrl_char() returns the input character
> unmodified.  So the behavior after the patch is the same as if the
> call to dispatch_ctrl_char() was guarded by the key_is_command_char()
> predicate, before the patch.

Sorry, you are right.