This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Explicitly call rl_resize_terminal() in TUI's SIGWINCH handler
- From: Pedro Alves <palves at redhat dot com>
- To: Patrick Palka <patrick at parcs dot ath dot cx>, gdb-patches at sourceware dot org
- Date: Thu, 23 Apr 2015 12:07:01 +0100
- Subject: Re: [PATCH] Explicitly call rl_resize_terminal() in TUI's SIGWINCH handler
- Authentication-results: sourceware.org; auth=none
- References: <1429746560-16979-1-git-send-email-patrick at parcs dot ath dot cx>
On 04/23/2015 12:49 AM, Patrick Palka wrote:
> (I am looking into syncing our copy of readline to the latest version,
> 6.3.)
>
> In readline 6.3, the semantics of SIGWINCH handling has changed.
> When a SIGWINCH signal is raised, readline's rl_sigwinch_handler() now
> does not immediately call rl_resize_terminal(). Instead it sets a flag
> that is checked by RL_CHECK_SIGNALS() at a point where readline has
> control, and calls rl_resize_terminal() if said flag is set.
>
> This change is item (c) in https://cnswww.cns.cwru.edu/php/chet/readline/CHANGES
>
> c. Fixed a bug that caused readline to try and run code to modify its idea
> of the screen size in a signal handler context upon receiving a SIGWINCH.
>
> This change in behavior is important to us because TUI's
> tui_sigwinch_handler() relies on the assumption that by the time it's
> called, readline will have updated its knowledge of the terminal
> dimensions via rl_resize_terminal(). Since this assumption no longer
> holds true, TUI's SIGWINCH handling does not work correctly with
> readline 6.3.
>
> To fix this issue this patch makes TUI explicitly call
> rl_resize_terminal() in tui_async_resize_screen() at the point where
> current terminal dimensions are needed. (We could call it in
> tui_sigwinch_handler too, but since readline avoids doing it, we are
> probably safer off avoiding to call it in signal handler context as
> well.) After this change, SIGWINCH handling continues to work properly
> with both readline 6.2 and 6.3.
OK with ChangeLog entry. I wonder if this fixes PR18155?
I think we should call this and update gdb's sense of the screen
size even with the tui disabled though. I currently see, outside
the TUI:
(gdb) show height
Number of lines gdb thinks are in a page is 45.
*resize*
(gdb) show height
Number of lines gdb thinks are in a page is 45.
* switch TUI on and off *
(gdb) show height
Number of lines gdb thinks are in a page is 27.
>
> Since we no longer need it, we could now explicitly disable readline's
> SIGWINCH handler by setting rl_handle_sigwinch to zero early on in the
> program startup but I can't seem to find a good spot to place this
> assignment (the first call to rl_initialize() occurs in
> tui_initialize_readline() so the assignment should occur before then),
> and the handler is harmless anyway.
ITYM rl_catch_sigwinch:
/* If non-zero, readline will install a signal handler for SIGWINCH
that also attempts to call any calling application's SIGWINCH signal
handler. Note that the terminal is not cleaned up before the
application's signal handler is called; use rl_cleanup_after_signal()
to do that. */
extern int rl_catch_sigwinch;
Maybe in init_page_info, and probably move the init_page_info call
to gdb_init directly for clarity. (I found that with "rbreak rl_*".)
Thanks,
Pedro Alves