Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src libcurses: fix wrong tab width for addch
details: https://anonhg.NetBSD.org/src/rev/94e74cc2460d
branches: trunk
changeset: 959422:94e74cc2460d
user: rillig <rillig%NetBSD.org@localhost>
date: Sat Feb 13 14:30:37 2021 +0000
description:
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.
diffstat:
lib/libcurses/addbytes.c | 62 +++++++------------------------
tests/lib/libcurses/check_files/addch.chk | 4 +-
tests/lib/libcurses/tests/addch | 10 +++--
3 files changed, 22 insertions(+), 54 deletions(-)
diffs (276 lines):
diff -r ad65255517c3 -r 94e74cc2460d lib/libcurses/addbytes.c
--- a/lib/libcurses/addbytes.c Sat Feb 13 13:00:16 2021 +0000
+++ b/lib/libcurses/addbytes.c Sat Feb 13 14:30:37 2021 +0000
@@ -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 @@
#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 @@
/*
* 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 @@
/*
* _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 @@
_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 @@
#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 @@
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 @@
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 @@
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 @@
#endif
if (!(win->flags & __SCROLLOK))
return ERR;
- PSYNCH_OUT;
scroll(win);
- PSYNCH_IN;
} else {
(*y)++;
}
@@ -361,27 +344,21 @@
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 @@
for (i = 0; i < newx; i++) {
if (wadd_wch(win, &cc) == ERR)
return ERR;
- (*x)++;
}
return OK;
}
@@ -433,9 +409,7 @@
if (*y == win->scr_b) {
if (!(win->flags & __SCROLLOK))
return ERR;
- PSYNCH_OUT;
scroll(win);
- PSYNCH_IN;
} else {
(*y)++;
}
@@ -498,16 +472,13 @@
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 @@
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) {
diff -r ad65255517c3 -r 94e74cc2460d tests/lib/libcurses/check_files/addch.chk
--- a/tests/lib/libcurses/check_files/addch.chk Sat Feb 13 13:00:16 2021 +0000
+++ b/tests/lib/libcurses/check_files/addch.chk Sat Feb 13 14:30:37 2021 +0000
@@ -1,2 +1,2 @@
-smsotrmsocup6;4Xsmsosmulermsormulcup7;17X8
-0123456 8
+smsotrmsocup6;4Xsmsosmulermsormulcup7;9X8
+0123456 8
diff -r ad65255517c3 -r 94e74cc2460d tests/lib/libcurses/tests/addch
--- a/tests/lib/libcurses/tests/addch Sat Feb 13 13:00:16 2021 +0000
+++ b/tests/lib/libcurses/tests/addch Sat Feb 13 14:30:37 2021 +0000
@@ -6,17 +6,19 @@
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`
Home |
Main Index |
Thread Index |
Old Index