tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

color curses leakage



hi-

    I'm seeing issues with pkgsrc mail/mutt under NetBSD 11
displaying text with the wrong colors.  The terminal/xterm
default colors are leaking through in places where they are
not supposed to!   If I recompile mail/mutt to use ncurses,
then it displays properly -- so the bugs are in NetBSD libcurses.

    Working with NetBSD libcurses from trunk, I've tracked the
the issue down to two main issues:

 - the handling of "orig_pair" in color.c
 - the handling of "exit_attribute_mode" in refresh.c

I have fixes for both that I'm hoping someone more familiar
with NetBSD curses can double check so we can get this fixed.


for "orig_pair"/color.c:
  - tput() of "orig_pair" resets the colors to terminal default.
    the only time you'd need to do this is when assume_default_colors()
    is used to change curses' default white on black (from
    __default_pair) with -1 to access one or both of the
    terminal default colors.

    start_colors():
      I removed tput() of "orig_pair" from start_color() and
      replaced it with a call to __unset_color(curscr) right 
      after setting __using_color = 1 (to ensure we are in a
      known state).   I also moved the tput() of "orig_colors"
      later (after we know we are not going to return(ERR)).

    __unset_color(): I added a helper function unset_color_pair()
      to centralize the handling of "orig_pair" across the
      COLOR_ANSI and COLOR_OTHER cases.  It only tputs() "orig_pair"
      if either __default_pair.fore or __default_pair.back is -1.

for "exit_attribute_mode"/refresh.c:
  - tput() of "exit_attribute_mode" resets the colors to terminal default.
    we need to add "_cursesi_screen->curpair = -1" to let curses know the
    currently set color has changed when "exit_attribute_mode" is used.


Also, I think init_pair() is supposed to fail if you call it
with -1 for a color and you have not called assume_default_colors().
At least that is what ncurses' init_pair() does.  I didn't address this.

diff relative to trunk below.

chuck


Index: color.c
===================================================================
RCS file: /cvsroot/src/lib/libcurses/color.c,v
retrieving revision 1.51
diff -u -r1.51 color.c
--- color.c	2 Sep 2026 04:20:20 -0000	1.51
+++ color.c	31 Oct 2026 21:16:45 -0000
@@ -124,14 +124,6 @@
 	_cursesi_screen->COLOR_PAIRS = COLOR_PAIRS;
 	_cursesi_screen->curpair = -1;
 
-	/* Reset terminal colour and colour pairs. */
-	if (orig_colors != NULL)
-		tputs(orig_colors, 0, __cputchar);
-	if (orig_pair != NULL) {
-		tputs(orig_pair, 0, __cputchar);
-		curscr->wattr &= _cursesi_screen->mask_op;
-	}
-
 	/* Type of colour manipulation - ANSI/TEK/HP/other */
 	if (set_a_foreground != NULL && set_a_background != NULL)
 		_cursesi_screen->color_type = COLOR_ANSI;
@@ -144,6 +136,10 @@
 	else
 		return(ERR);		/* Unsupported colour method */
 
+	/* Reset terminal colour and colour pairs. */
+	if (orig_colors != NULL)
+		tputs(orig_colors, 0, __cputchar);
+
 #ifdef DEBUG
 	__CTRACE(__CTRACE_COLOR, "start_color: COLORS = %d, COLOR_PAIRS = %d",
 	    COLORS, COLOR_PAIRS);
@@ -240,6 +236,7 @@
 	    __default_pair.flags;
 
 	__using_color = 1;
+	__unset_color(curscr);
 
 	/* Set all positions on all windows to curses default colours. */
 	for (wlp = _cursesi_screen->winlistp; wlp != NULL; wlp = wlp->nextp) {
@@ -585,6 +582,42 @@
 	curscr->wattr &= ~__COLOR;
 	curscr->wattr |= attr & __COLOR;
 }
+/*
+ * unset_color_pair --
+ *	Reset colors to the defaults.  If start_color() has been
+ *	called, then we use the colors stored in __default_pair
+ *	(white on black, if not changed by the application).  If
+ *	either color is -1 (indicates we should use the terminal
+ *	default for that value) we emit orig_pair to reset both
+ *	foreground and background to the terminal default and then
+ *	override the terminal default for any color that is not -1.
+ *	(-1 values can be established with the use_default_colors()
+ *	 or assume_default_colors() APIs.)
+ *
+ *	If start_color() has not been called, we just use orig_pair.
+ */
+static void
+unset_color_pair(WINDOW *win, const char *set_fg, const char *set_bg)
+{
+	if (__using_color) {
+		/* emit orig_pair if terminal default is used */
+		if (__default_pair.fore == -1 || __default_pair.back == -1) {
+			if (orig_pair)
+				tputs(orig_pair, 0, __cputchar);
+		}
+		if (__default_pair.fore >= 0 && set_fg)
+			tputs(tiparm(set_fg, (int)__default_pair.fore),
+				0, __cputchar);
+		if (__default_pair.back >= 0 && set_bg)
+			tputs(tiparm(set_bg, (int)__default_pair.back),
+				0, __cputchar);
+	} else {
+		if (orig_pair)
+			tputs(orig_pair, 0, __cputchar);
+	}
+	win->wattr &= __mask_op;
+	curscr->wattr &= __mask_op;
+}
 
 /*
  * __unset_color --
@@ -597,11 +630,9 @@
 	switch (_cursesi_screen->color_type) {
 	/* Clear ANSI foreground and background colours */
 	case COLOR_ANSI:
-		if (orig_pair != NULL) {
-			tputs(orig_pair, 0, __cputchar);
-			win->wattr &= __mask_op;
-			curscr->wattr &= __mask_op;
-		}
+		unset_color_pair(win,
+			t_set_a_foreground(_cursesi_screen->term),
+			t_set_a_background(_cursesi_screen->term));
 		break;
 	case COLOR_HP:
 		/* XXX: need to support HP style */
@@ -610,10 +641,9 @@
 		/* XXX: need to support Tek style */
 		break;
 	case COLOR_OTHER:
-		if (orig_pair != NULL) {
-			tputs(orig_pair, 0, __cputchar);
-			win->wattr &= __mask_op;
-		}
+		unset_color_pair(win,
+			t_set_foreground(_cursesi_screen->term),
+			t_set_background(_cursesi_screen->term));
 		break;
 	}
 
Index: refresh.c
===================================================================
RCS file: /cvsroot/src/lib/libcurses/refresh.c,v
retrieving revision 1.137
diff -u -r1.137 refresh.c
--- refresh.c	2 Sep 2026 04:20:20 -0000	1.137
+++ refresh.c	31 Oct 2026 21:16:46 -0000
@@ -880,6 +880,8 @@
 	 */
 	if (off & __TERMATTR && exit_attribute_mode != NULL) {
 		tputs(exit_attribute_mode, 0, __cputchar);
+		/* exit_attribute_mode resets the color to terminal default */
+		_cursesi_screen->curpair = -1;
 		curscr->wattr &= __mask_me;
 		off &= __mask_me;
 	}
@@ -2093,6 +2095,7 @@
 	 */
 	if (curscr->wattr & __TERMATTR && isms) {
 		tputs(exit_attribute_mode, 0, __cputchar);
+		_cursesi_screen->curpair = -1;  /* tputs just reset it */
 		curscr->wattr &= __mask_me;
 	}
 	/* Don't leave the screen with altcharset set (don't check ms). */


Home | Main Index | Thread Index | Old Index