[git commit] lineedit: fix two bugs in SIGWINCH signal handling

Denys Vlasenko vda.linux at googlemail.com
Sun Nov 27 21:25:07 UTC 2016


commit: https://git.busybox.net/busybox/commit/?id=bff71d3b9d4244abc5182c38d0a98f4913188391
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

(1) restore entire sigaction, not only signal handler function
(2) do not use stdio when not sure WINCH did not interrupt a printf() or such.

function                                             old     new   delta
cmdedit_setwidth                                       -      81     +81
read_line_input                                     3682    3722     +40
lineedit_read_key                                    138     155     +17
put_prompt                                            55      51      -4
win_changed                                           93      47     -46
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 2/2 up/down: 138/-50)            Total: 88 bytes

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 libbb/lineedit.c | 70 +++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 41 insertions(+), 29 deletions(-)

diff --git a/libbb/lineedit.c b/libbb/lineedit.c
index 4d7828c..7a1b543 100644
--- a/libbb/lineedit.c
+++ b/libbb/lineedit.c
@@ -129,8 +129,7 @@ static const char null_str[] ALIGN1 = "";
 struct lineedit_statics {
 	line_input_t *state;
 
-	volatile unsigned cmdedit_termw; /* = 80; */ /* actual terminal width */
-	sighandler_t previous_SIGWINCH_handler;
+	unsigned cmdedit_termw; /* = 80; */ /* actual terminal width */
 
 	unsigned cmdedit_x;        /* real x (col) terminal position */
 	unsigned cmdedit_y;        /* pseudoreal y (row) terminal position */
@@ -155,15 +154,22 @@ struct lineedit_statics {
 	unsigned num_matches;
 #endif
 
+	unsigned SIGWINCH_saved;
+	volatile unsigned SIGWINCH_count;
+	volatile smallint ok_to_redraw;
+
 #if ENABLE_FEATURE_EDITING_VI
 # define DELBUFSIZ 128
-	CHAR_T *delptr;
 	smallint newdelflag;     /* whether delbuf should be reused yet */
+	CHAR_T *delptr;
 	CHAR_T delbuf[DELBUFSIZ];  /* a place to store deleted characters */
 #endif
 #if ENABLE_FEATURE_EDITING_ASK_TERMINAL
 	smallint sent_ESC_br6n;
 #endif
+
+	/* Largish struct, keeping it last results in smaller code */
+	struct sigaction SIGWINCH_handler;
 };
 
 /* See lineedit_ptr_hack.c */
@@ -172,7 +178,6 @@ extern struct lineedit_statics *const lineedit_ptr_to_statics;
 #define S (*lineedit_ptr_to_statics)
 #define state            (S.state           )
 #define cmdedit_termw    (S.cmdedit_termw   )
-#define previous_SIGWINCH_handler (S.previous_SIGWINCH_handler)
 #define cmdedit_x        (S.cmdedit_x       )
 #define cmdedit_y        (S.cmdedit_y       )
 #define cmdedit_prmt_len (S.cmdedit_prmt_len)
@@ -434,14 +439,11 @@ static void beep(void)
 
 static void put_prompt(void)
 {
-	unsigned w;
-
 	fputs(cmdedit_prompt, stdout);
 	fflush_all();
 	cursor = 0;
-	w = cmdedit_termw; /* read volatile var once */
-	cmdedit_y = cmdedit_prmt_len / w; /* new quasireal y */
-	cmdedit_x = cmdedit_prmt_len % w;
+	cmdedit_y = cmdedit_prmt_len / cmdedit_termw; /* new quasireal y */
+	cmdedit_x = cmdedit_prmt_len % cmdedit_termw;
 }
 
 /* Move back one character */
@@ -513,13 +515,11 @@ static void input_backward(unsigned num)
 			put_cur_glyph_and_inc_cursor();
 	} else {
 		int lines_up;
-		unsigned width;
 		/* num = chars to go back from the beginning of current line: */
 		num -= cmdedit_x;
-		width = cmdedit_termw; /* read volatile var once */
 		/* num=1...w: one line up, w+1...2w: two, etc: */
-		lines_up = 1 + (num - 1) / width;
-		cmdedit_x = (width * cmdedit_y - num) % width;
+		lines_up = 1 + (num - 1) / cmdedit_termw;
+		cmdedit_x = (cmdedit_termw * cmdedit_y - num) % cmdedit_termw;
 		cmdedit_y -= lines_up;
 		/* go to 1st column; go up */
 		printf("\r" ESC"[%uA", lines_up);
@@ -1978,28 +1978,29 @@ static void parse_and_put_prompt(const char *prmt_ptr)
 }
 #endif
 
-static void cmdedit_setwidth(unsigned w, int redraw_flg)
+static void cmdedit_setwidth(int redraw_flg)
 {
-	cmdedit_termw = w;
+	get_terminal_width_height(STDIN_FILENO, &cmdedit_termw, NULL);
 	if (redraw_flg) {
 		/* new y for current cursor */
-		int new_y = (cursor + cmdedit_prmt_len) / w;
+		int new_y = (cursor + cmdedit_prmt_len) / cmdedit_termw;
 		/* redraw */
 		redraw((new_y >= cmdedit_y ? new_y : cmdedit_y), command_len - cursor);
 		fflush_all();
 	}
 }
 
-static void win_changed(int nsig)
+static void win_changed(int nsig UNUSED_PARAM)
 {
-	int sv_errno = errno;
-	unsigned width;
-
-	get_terminal_width_height(0, &width, NULL);
-//FIXME: cmdedit_setwidth() -> redraw() -> printf() -> KABOOM! (we are in signal handler!)
-	cmdedit_setwidth(width, /*redraw_flg:*/ nsig);
-
-	errno = sv_errno;
+	if (S.ok_to_redraw) {
+		/* We are in read_key(), safe to redraw immediately */
+		int sv_errno = errno;
+		cmdedit_setwidth(/*redraw_flg:*/ 1);
+		errno = sv_errno;
+	} else {
+		/* Signal main loop that redraw is necessary */
+		S.SIGWINCH_count++;
+	}
 }
 
 static int lineedit_read_key(char *read_key_buffer, int timeout)
@@ -2018,7 +2019,9 @@ static int lineedit_read_key(char *read_key_buffer, int timeout)
 		 *
 		 * Note: read_key sets errno to 0 on success.
 		 */
+		S.ok_to_redraw = 1;
 		ic = read_key(STDIN_FILENO, read_key_buffer, timeout);
+		S.ok_to_redraw = 0;
 		if (errno) {
 #if ENABLE_UNICODE_SUPPORT
 			if (errno == EAGAIN && unicode_idx != 0)
@@ -2355,9 +2358,11 @@ int FAST_FUNC read_line_input(line_input_t *st, const char *prompt, char *comman
 	ask_terminal();
 
 	/* Install window resize handler (NB: after *all* init is complete) */
-//FIXME: save entire sigaction!
-	previous_SIGWINCH_handler = signal(SIGWINCH, win_changed);
-	win_changed(0); /* get initial window size */
+	S.SIGWINCH_handler.sa_handler = win_changed;
+	S.SIGWINCH_handler.sa_flags = SA_RESTART;
+	sigaction(SIGWINCH, &S.SIGWINCH_handler, &S.SIGWINCH_handler);
+
+	cmdedit_setwidth(/*redraw_flg:*/ 0); /* get initial window size */
 
 	read_key_buffer[0] = 0;
 	while (1) {
@@ -2370,6 +2375,13 @@ int FAST_FUNC read_line_input(line_input_t *st, const char *prompt, char *comman
 		 * in one place.
 		 */
 		int32_t ic, ic_raw;
+		unsigned count;
+
+		count = S.SIGWINCH_count;
+		if (S.SIGWINCH_saved != count) {
+			S.SIGWINCH_saved = count;
+			cmdedit_setwidth(/*redraw_flg:*/ 1);
+		}
 
 		fflush_all();
 		ic = ic_raw = lineedit_read_key(read_key_buffer, timeout);
@@ -2808,7 +2820,7 @@ int FAST_FUNC read_line_input(line_input_t *st, const char *prompt, char *comman
 	/* restore initial_settings */
 	tcsetattr_stdin_TCSANOW(&initial_settings);
 	/* restore SIGWINCH handler */
-	signal(SIGWINCH, previous_SIGWINCH_handler);
+	sigaction_set(SIGWINCH, &S.SIGWINCH_handler);
 	fflush_all();
 
 	len = command_len;


More information about the busybox-cvs mailing list