This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Ensure rl_get_screen_size() returns the actual terminal dimensions
- From: Pedro Alves <palves at redhat dot com>
- To: Patrick Palka <patrick at parcs dot ath dot cx>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 04 Sep 2014 18:51:44 +0100
- Subject: Re: [PATCH] Ensure rl_get_screen_size() returns the actual terminal dimensions
- Authentication-results: sourceware.org; auth=none
- References: <1409534708-31981-1-git-send-email-patrick at parcs dot ath dot cx> <54084361 dot 1020906 at redhat dot com> <54084454 dot 5060207 at redhat dot com> <CA+C-WL9g9fp5+Gu5vSPyYebTB90PmQC7astD6uxfRz0Kwrg+5Q at mail dot gmail dot com>
On 09/04/2014 12:58 PM, Patrick Palka wrote:
>> readline's rl_sigwinch_handler already call rl_resize_terminal, and then
>> chains into the TUI's handler. So, why do we need this?
> Sometimes we overwrite readline's screen size by calling
> tui_update_gdb_sizes() like done in tui_enable(). When that's done
> rl_get_screen_size() will not return the real terminal dimensions, but
> rather the dimensions that were set by tui_update_gdb_sizes(). So in
> the following scenario the TUI will get resized to the wrong
> dimensions:
>
> 0. Run GDB.
> 1. Enter TUI mode.
> 2. Exit TUI mode.
> 3. Resize the terminal.
> 4. Enter TUI mode again.
> 5. Press a key so that the TUI will resize itself.
>
> Here we resize the terminal (causing RL's SIGWINCH handler to run,
> updating RL's idea of the terminal size). Then we enter TUI mode
> which calls tui_update_gdb_sizes() which resets RL's idea of the
> terminal size. Then we call rl_get_screen_size() in tui_getc() which
> returns not the terminal dimensions but the dimensions of the TUI's
> command window as set by tui_update_gdb_sizes(). So in the end the
> TUI gets resized to incorrect dimensions.
Hmm, OK, but... I think the story is just unraveling though. :-)
E.g., trying with your patch, I do:
0. Run GDB.
1. Enter TUI mode.
2. "show height" -> 15. Looks legit.
3. Exit TUI mode.
4. "show height" -> 45
5. Resize the terminal to something smaller than 45 lines.
6. "show height" -> still 45 (!?!?)
7. Enter TUI mode again. I have now about 3 visible lines in my cmd window.
8. "show height" -> 15 (!?!?)
9. Exit TUI mode.
A. "show height" -> _14_ (looks like it's the legit size now.)
B. Enter TUI mode again. I still still about 3 lines height.
C. "show height" -> _3_ (finally ??)
The screen size handling looks seriously confused.
Hmm, if I instead enter/exit/enter in a row, then the height
looks reasonable, like:
0. Run GDB.
1. "show height" -> 45
2. Resize the terminal to something smaller than 45 lines.
3. Enter TUI mode.
4. Exit TUI mode.
5. Enter TUI mode immediately again.
6. "show height" -> 3. Looks legit.
7. Exit TUI mode.
8. "show height" -> 14 (looks legit)
I notice we don't really need the TUI to see misbehavior, though:
0. Run GDB.
1. "show height" -> 45
2. Resize the terminal to something smaller than 45 lines.
3. "show height" -> still 45 (!?!?)
Sounds like GDB should have a SIGWINCH handler even if not running
the TUI, that marks the height as invalid, so the next time GDB needs
the height, it's refetched from readline.
>
> Also, when we are blocking inside a secondary prompt, readline's
> SIGWINCH handler does not seem to trigger for some reason. So when we
> resize the window during a secondary prompt, our SIGWINCH handler gets
> triggered but readline's handler doesn't which means that the
> dimensions returned by rl_get_screen_size() will be wrong. I'm not
> really sure why this happens.
>
> Explicitly calling rl_resize_terminal() fixes both of these issues.
So, it actually sounds like we _should_ be calling rl_resize_terminal
to get the full "root" window's sizes, and then _always_ end
up calling tui_update_gdb_sizes -> rl_set_screen_size ?
That, and maybe set rl_catch_sigwinch=1 as it sounds like
readline's signal handler would end up useless.
Thanks,
Pedro Alves