[PATCH 2/3] Update our idea of our terminal's dimensions even outside of TUI

Pedro Alves palves@redhat.com
Mon Apr 27 16:35:00 GMT 2015


On 04/24/2015 01:53 AM, Patrick Palka wrote:
> When in the CLI, GDB's "width" and "height" variables are not kept in sync
> when the underlying terminal gets resized.
> 
> This patch fixes this issue by making sure sure to update GDB's "width"
> and "height" variables in the !tui_active case of our SIGWINCH handler.
> 
> gdb/ChangeLog:
> 
> 	* tui/tui-win.c (tui_sigwinch_handler): Remove now-stale comment.
> 	(tui_sigwinch_handler): Still update our idea of
> 	the terminal's width and height even when TUI is not active.

(A next step for a rainy day would be to more the signal handler to
core code, and make this work even when the TUI is not compiled in.)

> @@ -850,15 +844,26 @@ tui_sigwinch_handler (int signal)
>  static void
>  tui_async_resize_screen (gdb_client_data arg)
>  {
> -  if (!tui_active)
> -    return;
> -
>    rl_resize_terminal ();
> -  tui_resize_all ();
> -  tui_refresh_all_win ();
> -  tui_update_gdb_sizes ();
> -  tui_set_win_resized_to (FALSE);
> -  tui_redisplay_readline ();
> +
> +  if (!tui_active)
> +    {
> +      int screen_height, screen_width;
> +
> +      rl_get_screen_size (&screen_height, &screen_width);
> +      set_screen_width_and_height (screen_width, screen_height);
> +
> +      /* win_resized will be untoggled and the windows resized in the next call
> +	 to tui_enable().  */

Hmm, can we please avoid "untoggle"?  I'm not a native speaker, but it
game me pause, as assuming toggle is x^=1, then untoggle is x^=1 too,
thus toggle==untoggle.  :-)  I'd rather use "unset".

OK with that change.

FWIW, the comment gives a bit of pause.  I'd suggest something like this
instead:

     /* win_resized is left set so that the next call to tui_enable
        resizes windows.  */

> +    }
> +  else
> +    {
> +      tui_resize_all ();
> +      tui_refresh_all_win ();
> +      tui_update_gdb_sizes ();
> +      tui_set_win_resized_to (FALSE);
> +      tui_redisplay_readline ();

A preexisting problem (thus not be fixed by this patch), but
I note that this seems racy.  If another SIGWINCH comes in after
between tui_resize_all and tui_set_win_resized_to, we'll clear
tui_set_win_resized_to and lose the need to resize in tui_enable:

  /* Resize windows before anything might display/refresh a
     window.  */
  if (tui_win_resized ())
    {
      tui_resize_all ();
      tui_set_win_resized_to (FALSE);
    }

That's why the mainline code that handles a signal should always clear
the signal handler's control variable before actually reacting to
it, like:

      tui_set_win_resized_to (FALSE);
      tui_resize_all ();
      tui_refresh_all_win ();
      tui_update_gdb_sizes ();
      tui_redisplay_readline ();

Thanks,
Pedro Alves



More information about the Gdb-patches mailing list