Re: lib/56224: libcurses: adding character to last line of non-scrolling window wraps instead of

[Michael Forney <mforney%mforney.org@localhost>]

The following reply was made to PR lib/56224; it has been noted by GNATS.

From: Michael Forney <mforney%mforney.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: lib/56224: libcurses: adding character to last line of
 non-scrolling window wraps instead of 
Date: Wed, 09 Jun 2021 20:54:03 -0700

 Brett Lymn <blymn%internode.on.net@localhost> wrote:
 >  I have just committed a fix for this, please update libcurses, rebuild
 >  and reinstall it.
 
 Thanks for the patch, I can confirm that this does solve the issue
 for the application in question (catgirl).
 
 However, I did a bit of investigation myself, and have a few comments:
 
 1. It seems that now the string is truncated by one extra character
 (the last column is empty). Since this last character doesn't extend
 beyond the end of the line, my expectation is that it should not
 be truncated. And indeed, this is how it behaves in ncurses.
 
 2. I was thinking that this needed to be fixed in _cursesi_addwchar,
 not _cursesi_addwbytes. Otherwise, addwstr and addch at the last
 column still wrap to column 0 (though, addwstr already truncates
 since it returns on the first ERR, whereas _curses_waddbytes ignores
 all errors except for the last character).
 
 3. This seems incorrect to me:
 
 	n =3D (int)mbrtowc(&wc, bytes, (size_t)count, &st);
 	...
 	/* if scrollok is false and we are at the bottom of
 	 * screen and this character would take us past the
 	 * end of the line then we are done.
 	 */
 	if ((win->curx + n >=3D win->maxx) &&
 	    (!(win->flags & __SCROLLOK)) &&
 	    (win->cury =3D=3D win->scr_b))
 		break;
 
 Here, n is the number of bytes in a multi-byte character, *not* the
 column width of that character. For example, consider this modified
 test program:
 
 	#include <curses.h>
 	#include <locale.h>
 =09
 	int main(void) {
 		setlocale(LC_ALL, "C.UTF-8");
 		initscr();
 		noecho();
 		raw();
 		do {
 			clear();
 			mvaddstr(LINES - 1, COLS - 7, "foo=E2=86=92=E2=86=92=E2=86=92");
 			refresh();
 		} while (getch() !=3D 'q');
 		endwin();
 		return 0;
 	}
 
 The string gets truncated after the first '=E2=86=92' (U+2192, 3 bytes in
 UTF-8), even though there are 3 more columns available in the line.
 If you replace '=E2=86=92' with '=C2=B5' (U+03BC, 2 bytes in UTF-8), it get=
 s
 truncated after the second one. If you replace it with '*', it does
 not get truncated.
 
 Below is an alternative fix I've been testing that seems to solve
 these problems. The first thing it does is make _cursesi_waddbytes
 break from the loop and return ERR when any of the _cursesi_addwchar
 calls returns ERR (not just the last one). This is consistent with
 waddnwstr. The second thing it does is delay wrapping the cursor x
 position back to zero until after we return ERR if scrolling is not
 allowed.
 
 diff --git a/lib/libcurses/addbytes.c b/lib/libcurses/addbytes.c
 index 0a85ae76c3ad..8533a60ed752 100644
 --- a/lib/libcurses/addbytes.c
 +++ b/lib/libcurses/addbytes.c
 @@ -141,7 +141,7 @@ _cursesi_waddbytes(WINDOW *win, const char *bytes, int =
 count, attr_t attr,
  #ifdef HAVE_WCHAR
  	(void)memset(&st, 0, sizeof(st));
  #endif
 -	while (count > 0) {
 +	while (count > 0 && err =3D=3D OK) {
  #ifndef HAVE_WCHAR
  		c =3D *bytes++;
  #ifdef DEBUG
 @@ -241,7 +241,6 @@ _cursesi_addbyte(WINDOW *win, __LINE **lp, int *y, int =
 *x, int c,
  #endif
 =20
  	if (char_interp && ((*lp)->flags & __ISPASTEOL)) {
 -		*x =3D 0;
  		(*lp)->flags &=3D ~__ISPASTEOL;
  		if (*y =3D=3D win->scr_b) {
  #ifdef DEBUG
 @@ -255,6 +254,7 @@ _cursesi_addbyte(WINDOW *win, __LINE **lp, int *y, int =
 *x, int c,
  		} else {
  			(*y)++;
  		}
 +		*x =3D 0;
  		*lp =3D win->alines[*y];
  		if (c =3D=3D '\n')
  			return OK;
 @@ -350,7 +350,6 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, in=
 t *x,
  			return OK;
  		case L'\n':
  			wclrtoeol(win);
 -			*x =3D 0;
  			(*lnp)->flags &=3D ~__ISPASTEOL;
  			if (*y =3D=3D win->scr_b) {
  				if (!(win->flags & __SCROLLOK))
 @@ -359,6 +358,7 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, in=
 t *x,
  			} else {
  				(*y)++;
  			}
 +			*x =3D 0;
  			return OK;
  		case L'\t':
  			cc.vals[0] =3D L' ';
 @@ -404,7 +404,6 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, in=
 t *x,
  	}
  	/* check for new line first */
  	if (char_interp && ((*lnp)->flags & __ISPASTEOL)) {
 -		*x =3D 0;
  		(*lnp)->flags &=3D ~__ISPASTEOL;
  		if (*y =3D=3D win->scr_b) {
  			if (!(win->flags & __SCROLLOK))
 @@ -413,6 +412,7 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, in=
 t *x,
  		} else {
  			(*y)++;
  		}
 +		*x =3D 0;
  		(*lnp) =3D win->alines[*y];
  		lp =3D &win->alines[*y]->line[*x];
  	}
 @@ -468,7 +468,6 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, in=
 t *x,
  		if (newx > *(*lnp)->lastchp)
  			*(*lnp)->lastchp =3D newx;
  		__touchline(win, *y, sx, (int) win->maxx - 1);
 -		sx =3D *x =3D 0;
  		if (*y =3D=3D win->scr_b) {
  			if (!(win->flags & __SCROLLOK))
  				return ERR;
 @@ -476,6 +475,7 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, in=
 t *x,
  		} else {
  			(*y)++;
  		}
 +		sx =3D *x =3D 0;
  		lp =3D &win->alines[*y]->line[0];
  		(*lnp) =3D win->alines[*y];
  	}
 @@ -556,14 +556,16 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, =
 int *x,
  		if (newx > *(*lnp)->lastchp)
  			*(*lnp)->lastchp =3D newx;
  		__touchline(win, *y, sx, (int) win->maxx - 1);
 -		*x =3D sx =3D 0;
  		if (*y =3D=3D win->scr_b) {
 -			if (!(win->flags & __SCROLLOK))
 +			if (!(win->flags & __SCROLLOK)) {
 +				*x =3D win->maxx - 1;
  				return ERR;
 +			}
  			scroll(win);
  		} else {
  			(*y)++;
  		}
 +		*x =3D sx =3D 0;
  		lp =3D &win->alines[*y]->line[0];
  		(*lnp) =3D win->alines[*y];
  	} else {