libcurses: fix wrong tab width for addch

In sysinst, the installation screen is indented with tabs.  Sysinst uses
msgc, which brings its own text layout engine.  This engine does not use
addbytes but addch.  In addch, the x position for each tab was advanced
twice as much as needed.  The menu items were thus not indented by 8
spaces but by 16, which caused an ugly line break in the German
translation.

This bug largely went unnoticed because most other applications use
addbytes instead, which worked fine all the time.  It had been
introduced somewhere between NetBSD 8.0 and NetBSD 9.0.

The code around this bug used aliased variables for win->curx and
win->cury a lot.  Getting this right is difficult and needs a thorough
test suite.  Even though libcurses has 201 tests, that is not nearly
enough to cover all the relations between the various functions in
libcurses that call each other, crossing API boundaries from internal
to external, doing character conversions on the way and juggling around
4 different types of characters (char, wchar_t, chtype, cchar_t).

The simplest fix was to remove all this aliasing, while keeping the
API the same.  If _cursesi_waddbytes is not considered part of the API,
it would be possible to replace px with win->curx in all places, same
for py and win->cury.

The complicated code with the aliasing may have been meant for
performance reasons, but it's hard to see any advantage if both points
of truth need to be synchronized all the time.

Libcurses can be built in 2 modes: with wide character support or
without (-DDISABLE_WCHAR).  The test suite only covers the variant with
wide characters.  The single-byte variant has to be tested manually.
Running sysinst with the single-byte libcurses produces the correct
layout.
This commit is contained in:
rillig 2021-02-13 14:30:37 +00:00
parent cb6917aa0d
commit 609d11bc47
3 changed files with 22 additions and 54 deletions

View File

@ -1,4 +1,4 @@
/* $NetBSD: addbytes.c,v 1.53 2021/02/06 19:41:14 rillig Exp $ */
/* $NetBSD: addbytes.c,v 1.54 2021/02/13 14:30:37 rillig Exp $ */
/*
* Copyright (c) 1987, 1993, 1994
@ -34,7 +34,7 @@
#if 0
static char sccsid[] = "@(#)addbytes.c 8.4 (Berkeley) 5/4/94";
#else
__RCSID("$NetBSD: addbytes.c,v 1.53 2021/02/06 19:41:14 rillig Exp $");
__RCSID("$NetBSD: addbytes.c,v 1.54 2021/02/13 14:30:37 rillig Exp $");
#endif
#endif /* not lint */
@ -46,16 +46,11 @@ __RCSID("$NetBSD: addbytes.c,v 1.53 2021/02/06 19:41:14 rillig Exp $");
#include <assert.h>
#endif
#define SYNCH_IN {y = win->cury; x = win->curx;}
#define SYNCH_OUT {win->cury = y; win->curx = x;}
#define PSYNCH_IN {*y = win->cury; *x = win->curx;}
#define PSYNCH_OUT {win->cury = *y; win->curx = *x;}
#ifndef _CURSES_USE_MACROS
/*
* addbytes --
* Add the character to the current position in stdscr.
* Add the characters to the current position in stdscr.
*/
int
addbytes(const char *bytes, int count)
@ -66,7 +61,7 @@ addbytes(const char *bytes, int count)
/*
* waddbytes --
* Add the character to the current position in the given window.
* Add the characters to the current position in the given window.
*/
int
waddbytes(WINDOW *win, const char *bytes, int count)
@ -111,7 +106,7 @@ __waddbytes(WINDOW *win, const char *bytes, int count, attr_t attr)
/*
* _cursesi_waddbytes --
* Add the character to the current position in the given window.
* Add the characters to the current position in the given window.
* if char_interp is non-zero then character interpretation is done on
* the byte (i.e. \n to newline, \r to carriage return, \b to backspace
* and so on).
@ -120,7 +115,7 @@ int
_cursesi_waddbytes(WINDOW *win, const char *bytes, int count, attr_t attr,
int char_interp)
{
int x, y, err;
int *py = &win->cury, *px = &win->curx, err;
__LINE *lp;
#ifdef HAVE_WCHAR
int n;
@ -141,8 +136,7 @@ _cursesi_waddbytes(WINDOW *win, const char *bytes, int count, attr_t attr,
#endif
err = OK;
SYNCH_IN;
lp = win->alines[y];
lp = win->alines[*py];
#ifdef HAVE_WCHAR
(void)memset(&st, 0, sizeof(st));
@ -152,13 +146,13 @@ _cursesi_waddbytes(WINDOW *win, const char *bytes, int count, attr_t attr,
c = *bytes++;
#ifdef DEBUG
__CTRACE(__CTRACE_INPUT, "ADDBYTES('%c', %x) at (%d, %d)\n",
c, attr, y, x);
c, attr, *py, *px);
#endif
err = _cursesi_addbyte(win, &lp, &y, &x, c, attr, char_interp);
err = _cursesi_addbyte(win, &lp, py, px, c, attr, char_interp);
count--;
#else
/*
* For wide-character support only, try and convert the
* For wide-character support only, try to convert the
* given string into a wide character - we do this because
* this is how ncurses behaves (not that I think this is
* actually the correct thing to do but if we don't do it
@ -177,21 +171,19 @@ _cursesi_waddbytes(WINDOW *win, const char *bytes, int count, attr_t attr,
break;
}
#ifdef DEBUG
__CTRACE(__CTRACE_INPUT,
"ADDBYTES WIDE(0x%x [%s], %x) at (%d, %d), ate %d bytes\n",
(unsigned)wc, unctrl((unsigned)wc), attr, y, x, n);
__CTRACE(__CTRACE_INPUT,
"ADDBYTES WIDE(0x%x [%s], %x) at (%d, %d), ate %d bytes\n",
(unsigned)wc, unctrl((unsigned)wc), attr, *py, *px, n);
#endif
cc.vals[0] = wc;
cc.elements = 1;
cc.attributes = attr;
err = _cursesi_addwchar(win, &lp, &y, &x, &cc, char_interp);
err = _cursesi_addwchar(win, &lp, py, px, &cc, char_interp);
bytes += n;
count -= n;
#endif
}
SYNCH_OUT;
#ifdef DEBUG
for (i = 0; i < win->maxy; i++) {
assert(win->alines[i]->sentinel == SENTINEL_VALUE);
@ -221,32 +213,25 @@ _cursesi_addbyte(WINDOW *win, __LINE **lp, int *y, int *x, int c,
switch (c) {
case '\t':
tabsize = win->screen->TABSIZE;
PSYNCH_OUT;
newx = tabsize - (*x % tabsize);
for (i = 0; i < newx; i++) {
if (waddbytes(win, blank, 1) == ERR)
return ERR;
(*x)++;
}
PSYNCH_IN;
return OK;
case '\n':
PSYNCH_OUT;
wclrtoeol(win);
PSYNCH_IN;
(*lp)->flags |= __ISPASTEOL;
break;
case '\r':
*x = 0;
win->curx = *x;
return OK;
case '\b':
if (--(*x) < 0)
*x = 0;
win->curx = *x;
return OK;
}
}
@ -266,9 +251,7 @@ _cursesi_addbyte(WINDOW *win, __LINE **lp, int *y, int *x, int c,
#endif
if (!(win->flags & __SCROLLOK))
return ERR;
PSYNCH_OUT;
scroll(win);
PSYNCH_IN;
} else {
(*y)++;
}
@ -361,27 +344,21 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, int *x,
case L'\b':
if (--*x < 0)
*x = 0;
win->curx = *x;
return OK;
case L'\r':
*x = 0;
win->curx = *x;
return OK;
case L'\n':
wclrtoeol(win);
PSYNCH_IN;
*x = 0;
(*lnp)->flags &= ~__ISPASTEOL;
if (*y == win->scr_b) {
if (!(win->flags & __SCROLLOK))
return ERR;
PSYNCH_OUT;
scroll(win);
PSYNCH_IN;
} else {
(*y)++;
}
PSYNCH_OUT;
return OK;
case L'\t':
cc.vals[0] = L' ';
@ -392,7 +369,6 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, int *x,
for (i = 0; i < newx; i++) {
if (wadd_wch(win, &cc) == ERR)
return ERR;
(*x)++;
}
return OK;
}
@ -433,9 +409,7 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, int *x,
if (*y == win->scr_b) {
if (!(win->flags & __SCROLLOK))
return ERR;
PSYNCH_OUT;
scroll(win);
PSYNCH_IN;
} else {
(*y)++;
}
@ -498,16 +472,13 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, int *x,
if (*y == win->scr_b) {
if (!(win->flags & __SCROLLOK))
return ERR;
PSYNCH_OUT;
scroll(win);
PSYNCH_IN;
} else {
(*y)++;
}
lp = &win->alines[*y]->line[0];
(*lnp) = win->alines[*y];
}
win->cury = *y;
/* add spacing character */
#ifdef DEBUG
@ -589,18 +560,13 @@ _cursesi_addwchar(WINDOW *win, __LINE **lnp, int *y, int *x,
if (*y == win->scr_b) {
if (!(win->flags & __SCROLLOK))
return ERR;
PSYNCH_OUT;
scroll(win);
PSYNCH_IN;
} else {
(*y)++;
}
lp = &win->alines[*y]->line[0];
(*lnp) = win->alines[*y];
win->curx = *x;
win->cury = *y;
} else {
win->curx = *x;
/* clear the remaining of the current character */
if (*x && *x < win->maxx) {

View File

@ -1,2 +1,2 @@
smsotrmsocup6;4Xsmsosmulermsormulcup7;17X8
0123456 8
smsotrmsocup6;4Xsmsosmulermsormulcup7;9X8
0123456 8

View File

@ -6,17 +6,19 @@ call OK refresh
call OK addch `\000\n`
# Somewhere between NetBSD 8.0 and 9.0, a bug was added to addch that
# doubled the spaces for a tab. Instead of 8 spaces, there are now 16.
# doubled the spaces for a tab. Instead of 8 spaces, there were 16.
# Fixed in NetBSD 9.99.80.
call OK addch `\000\t`
call2 6 16 getyx STDSCR # FIXME: must be 8, not 16
call2 6 8 getyx STDSCR
call OK addch `\0008`
call OK addch `\000\n`
# Somewhere between NetBSD 8.0 and 9.0, a bug was added to addch that
# doubled the spaces for a tab. Instead of 1 space, there are now 2.
# doubled the spaces for a tab. Instead of 1 space, there were 2.
# Fixed in NetBSD 9.99.80.
call OK addstr "0123456"
call OK addch `\000\t`
call2 7 9 getyx STDSCR
call2 7 8 getyx STDSCR
call OK addch `\0008`
call OK addch `\000\n`