[PATCH v3] lineedit: fix some tab completions written to stdout
Sertonix
sertonix at posteo.net
Fri Dec 13 12:18:00 UTC 2024
Sorry for not responding to the previous reply. I got stuck when I
realized that puts and fputs behave differently (puts adds a newline
at the end and fputs doesn't). So the suggestions you made didn't work
correctly.
On Fri Dec 13, 2024 at 1:06 PM CET, Natanael Copa wrote:
> From: Sertonix <sertonix at posteo.net>
>
> In fd47f056765 (lineedit: print prompt and editing operations to stderr)
> some output was left printing to stdout. This causes a race condition
> between stderr and stdout which in some cases leads to output written in
> the wrong places.
>
> Downstream issue: https://gitlab.alpinelinux.org/alpine/aports/-/issues/16566
> Fixes: fd47f056765
>
> function old new delta
> fputs_stderr - 12 +12
> put_cur_glyph_and_inc_cursor 182 178 -4
> put_prompt_custom 52 45 -7
> input_delete 161 154 -7
> draw_custom 84 77 -7
> BB_PUTCHAR 164 157 -7
> read_line_input 3612 3579 -33
> ------------------------------------------------------------------------------
> (add/remove: 1/0 grow/shrink: 0/6 up/down: 12/-65) Total: -53 bytes
> text data bss dec hex filename
> 824213 14108 2008 840329 cd289 busybox_old
> 824160 14108 2008 840276 cd254 busybox_unstripped
>
> Signed-off-by: Sertonix <sertonix at posteo.net>
> Signed-off-by: Natanael Copa <ncopa at alpinelinux.org>
> ---
>
> v2 -> v3:
> Introduce a fputs_stderr() func to reduce size.
>
> Denys, It would be nice if this could be applied and backported to
> 1_37_stable.
>
> Thanks!
>
>
> libbb/lineedit.c | 28 +++++++++++++++++-----------
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/libbb/lineedit.c b/libbb/lineedit.c
> index 151208c1c..21fd6037c 100644
> --- a/libbb/lineedit.c
> +++ b/libbb/lineedit.c
> @@ -345,6 +345,12 @@ static unsigned save_string(char *dst, unsigned maxsize)
> return i;
> }
> }
> +
> +static void fputs_stderr(const char *buf)
> +{
> + fputs(buf, stderr);
> +}
> +
> /* I thought just fputwc(c, stderr) would work. But no... */
> static void BB_PUTCHAR(wchar_t c)
> {
> @@ -354,7 +360,7 @@ static void BB_PUTCHAR(wchar_t c)
> ssize_t len = wcrtomb(buf, c, &mbst);
> if (len > 0) {
> buf[len] = '\0';
> - fputs(buf, stderr);
> + fputs_stderr(buf);
> }
> } else {
> /* In this case, c is always one byte */
> @@ -451,7 +457,7 @@ static void put_cur_glyph_and_inc_cursor(void)
> * have automargin (IOW: it is moving cursor to next line
> * by itself (which is wrong for VT-10x terminals)),
> * this will break things: there will be one extra empty line */
> - puts("\r"); /* + implicit '\n' */
> + bb_putchar_stderr('\r'); /* + implicit '\n' */
This needs to be fputs_stderr("\r\n")
> #else
> /* VT-10x terminals don't wrap cursor to next line when last char
> * on the line is printed - cursor stays "over" this char.
> @@ -505,7 +511,7 @@ static void put_prompt_custom(bool is_full)
> /* https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
> * says that shells must write $PSn to stderr, not stdout.
> */
> - fputs((is_full ? cmdedit_prompt : prompt_last_line), stderr);
> + fputs_stderr((is_full ? cmdedit_prompt : prompt_last_line));
> cursor = 0;
> cmdedit_y = cmdedit_prmt_len / cmdedit_termw; /* new quasireal y */
> cmdedit_x = cmdedit_prmt_len % cmdedit_termw;
> @@ -607,7 +613,7 @@ static void draw_custom(int y, int back_cursor, bool is_full)
> bb_putchar_stderr('\r');
> put_prompt_custom(is_full);
> put_till_end_and_adv_cursor();
> - fputs(SEQ_CLEAR_TILL_END_OF_SCREEN, stderr);
> + fputs_stderr(SEQ_CLEAR_TILL_END_OF_SCREEN);
> input_backward(back_cursor);
> }
>
> @@ -652,7 +658,7 @@ static void input_delete(int save)
> command_len--;
> put_till_end_and_adv_cursor();
> /* Last char is still visible, erase it (and more) */
> - fputs(SEQ_CLEAR_TILL_END_OF_SCREEN, stderr);
> + fputs_stderr(SEQ_CLEAR_TILL_END_OF_SCREEN);
> input_backward(cursor - j); /* back to old pos cursor */
> }
>
> @@ -1170,9 +1176,9 @@ static void showfiles(void)
> );
> }
> if (ENABLE_UNICODE_SUPPORT)
> - puts(printable_string(matches[n]));
> + fputs_stderr(printable_string(matches[n]));
> else
> - puts(matches[n]);
> + fputs_stderr(matches[n]);
There needs to be an additional bb_putchar_stderr('\n') here to compensate
the puts vs. fputs difference.
> }
> }
>
> @@ -1903,7 +1909,7 @@ static void ask_terminal(void)
> pfd.events = POLLIN;
> if (safe_poll(&pfd, 1, 0) == 0) {
> S.sent_ESC_br6n = 1;
> - fputs(ESC"[6n", stderr);
> + fputs_stderr(ESC"[6n");
> fflush_all(); /* make terminal see it ASAP! */
> }
> }
> @@ -2642,13 +2648,13 @@ int FAST_FUNC read_line_input(line_input_t *st, const char *prompt, char *comman
> /* Control-k -- clear to end of line */
> command_ps[cursor] = BB_NUL;
> command_len = cursor;
> - fputs(SEQ_CLEAR_TILL_END_OF_SCREEN, stderr);
> + fputs_stderr(SEQ_CLEAR_TILL_END_OF_SCREEN);
> break;
> case CTRL('L'):
> vi_case(CTRL('L')|VI_CMDMODE_BIT:)
> /* Control-l -- clear screen */
> /* cursor to top,left; clear to the end of screen */
> - fputs(ESC"[H" ESC"[J", stderr);
> + fputs_stderr(ESC"[H" ESC"[J");
> draw_full(command_len - cursor);
> break;
> #if MAX_HISTORY > 0
> @@ -3033,7 +3039,7 @@ int FAST_FUNC read_line_input(const char* prompt, char* command, int maxsize)
> /* https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
> * says that shells must write $PSn to stderr, not stdout.
> */
> - fputs(prompt, stderr);
> + fputs_stderr(prompt);
> fflush_all();
> if (!fgets(command, maxsize, stdin))
> return -1;
More information about the busybox
mailing list