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 <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Natanael Copa <[email protected]>
> ---
>
> 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;

_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox

Reply via email to