Patchwork [stable] chg: forward SIGWINCH to worker

login
register
mail settings
Submitter Jun Wu
Date April 16, 2016, 11:22 p.m.
Message ID <9e798e2ead7cc52a8a19.1460848943@x1c>
Download mbox | patch
Permalink /patch/14704/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - April 16, 2016, 11:22 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1460248132 -3600
#      Sun Apr 10 01:28:52 2016 +0100
# Node ID 9e798e2ead7cc52a8a199d2ef89cf05fc6af5645
# Parent  7b008530793234393f512275ebca12df4d9fd394
chg: forward SIGWINCH to worker

Before this patch, if the user uses chg and ncurses interface, resizing the
terminal window will mess up its content.

This patch fixes the issue by forwarding SIGWINCH to the worker process.
Jun Wu - April 16, 2016, 11:29 p.m.
I notice that setupsignalhandler() can be cleaned up a bit (remove
duplicated "sa.sa_flags = SA_RESTART;", "sa.sa_handler = forwardsignal;"
and add a comment for SIGHUP, SIGINT, besides, explain why handlestopsignal
is used instead of just forwardsignal).

I will do that after the freeze.

On 04/17/2016 12:22 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1460248132 -3600
> #      Sun Apr 10 01:28:52 2016 +0100
> # Node ID 9e798e2ead7cc52a8a199d2ef89cf05fc6af5645
> # Parent  7b008530793234393f512275ebca12df4d9fd394
> chg: forward SIGWINCH to worker
>
> Before this patch, if the user uses chg and ncurses interface, resizing the
> terminal window will mess up its content.
>
> This patch fixes the issue by forwarding SIGWINCH to the worker process.
>
> diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
> --- a/contrib/chg/chg.c
> +++ b/contrib/chg/chg.c
> @@ -397,6 +397,10 @@
>   	if (sigaction(SIGTERM, &sa, NULL) < 0)
>   		goto error;
>
> +	/* notify the worker about window resize events */
> +	sa.sa_flags = SA_RESTART;
> +	if (sigaction(SIGWINCH, &sa, NULL) < 0)
> +		goto error;
>   	/* propagate job control requests to worker */
>   	sa.sa_handler = forwardsignal;
>   	sa.sa_flags = SA_RESTART;
Yuya Nishihara - April 17, 2016, 1:50 p.m.
On Sun, 17 Apr 2016 00:22:23 +0100, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1460248132 -3600
> #      Sun Apr 10 01:28:52 2016 +0100
> # Node ID 9e798e2ead7cc52a8a199d2ef89cf05fc6af5645
> # Parent  7b008530793234393f512275ebca12df4d9fd394
> chg: forward SIGWINCH to worker

Confirmed the fix. Pushed to the committed repo, thanks!
Yuya Nishihara - April 17, 2016, 2:03 p.m.
On Sun, 17 Apr 2016 00:29:35 +0100, Jun Wu wrote:
> I notice that setupsignalhandler() can be cleaned up a bit (remove
> duplicated "sa.sa_flags = SA_RESTART;", "sa.sa_handler = forwardsignal;"
> and add a comment for SIGHUP, SIGINT, besides, explain why handlestopsignal
> is used instead of just forwardsignal).

Well, I made it verbose by intention so that I can easily see which signal is
handled by which function. I'd rather reorganize the first SIGHUP/SIGINT/SIGTERM
block, and add comment for it.

Patch

diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
--- a/contrib/chg/chg.c
+++ b/contrib/chg/chg.c
@@ -397,6 +397,10 @@ 
 	if (sigaction(SIGTERM, &sa, NULL) < 0)
 		goto error;
 
+	/* notify the worker about window resize events */
+	sa.sa_flags = SA_RESTART;
+	if (sigaction(SIGWINCH, &sa, NULL) < 0)
+		goto error;
 	/* propagate job control requests to worker */
 	sa.sa_handler = forwardsignal;
 	sa.sa_flags = SA_RESTART;