NetBSD-Bugs archive

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

lib/60501: lib/curses: wget_wch() returns KEY_CODE_YES for a character pushed with unget_wch()/ungetch()



>Number:         60501
>Category:       lib
>Synopsis:       lib/curses: wget_wch() returns KEY_CODE_YES for a character pushed
 with unget_wch()/ungetch()
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Jul 25 02:35:00 +0000 2026
>Originator:     Serhiy Storchaka
>Release:        NetBSD 10.1
>Environment:
System: NetBSD 10.1 amd64 (GENERIC), libcurses.so.9.1>Synopsis:	curses: wget_wch() reports an ungot ordinary character as a 
key code
>Description:

wget_wch() must return KEY_CODE_YES only for a function key (a KEY_* value)
and OK for an ordinary character.  After a character is pushed back with
unget_wch() (or ungetch()), wget_wch() instead returns KEY_CODE_YES for it,
so the caller cannot tell an ungot character from a real function key.

unget_wch() and ungetch() both append to the same list via __unget()
(getch.c); the list stores a bare wchar_t per entry, with no 
key-vs-character
tag.  wget_wch() then pops from it and always reports a key code (get_wch.c,
the unget branch):

     if (_cursesi_screen->unget_pos) {
         _cursesi_screen->unget_pos--;
         *ch = _cursesi_screen->unget_list[_cursesi_screen->unget_pos];
         if (__echoit) { ... wadd_wch(win, &wc); }
         return KEY_CODE_YES;        /* wrong for a pushed character */
     }

So every pushed value, an ordinary character from unget_wch() included, 
comes
back as KEY_CODE_YES.  X/Open Curses reserves KEY_CODE_YES for a key 
code, so
this breaks the contract for a pushed character.

(Found via CPython's curses module: window.get_wch() returns an int when
wget_wch() yields KEY_CODE_YES and a str otherwise, so unget_wch('a') +
get_wch() yields the int 97 instead of the string 'a'.  ncurses returns OK
for the same sequence.)

>How-To-Repeat:

     /* unget_wch_keycode.c
        cc unget_wch_keycode.c -lcurses && ./a.out < /dev/null */
     #include <curses.h>
     #include <locale.h>
     #include <stdio.h>
     int main(void) {
         wint_t wch = 0;
         int ret;
         setlocale(LC_ALL, "");
         initscr();
         unget_wch((wchar_t)0x61);        /* push ordinary character 
U+0061 */
         ret = wget_wch(stdscr, &wch);
         endwin();
         printf("wget_wch: ret=%s  wch=U+%04X\n",
             ret==KEY_CODE_YES ? "KEY_CODE_YES" : ret==OK ? "OK" : "ERR",
             (unsigned)wch);
         return 0;
     }

Observed on NetBSD 10.1 (libcurses.so.9.1):

     wget_wch: ret=KEY_CODE_YES  wch=U+0061

Expected:

     wget_wch: ret=OK  wch=U+0061

>Fix:

A key code and a wide character overlap (KEY_MIN..KEY_MAX is 0x101..0x240,
which also holds characters such as U+0141), so the class cannot be 
recovered
by value when popped; it must be recorded when pushed.  The patch tags each
unget entry:

   * a bool array (unget_key) parallels unget_list in struct __screen, 
and is
     allocated, grown and freed alongside it;
   * __unget() takes an "iskey" flag; unget_wch() passes false (it 
always pushes
     a character), ungetch() passes (c >= KEY_MIN && c <= KEY_MAX) (its 
argument
     is what getch() returned, where a character is a byte < KEY_MIN and 
cannot
     collide with a key);
   * wget_wch()'s unget branch returns KEY_CODE_YES only for an entry 
tagged as
     a key, and OK otherwise.

Verified: unget_wch() of 'a', of U+0141 (in the key range) and of U+20AC all
return OK; ungetch(KEY_LEFT) still returns KEY_CODE_YES; ungetch('Z') 
returns OK.

(The patch also fixes the byte counts in __unget()'s realloc-failure 
"lose the
oldest entry" fallback, which mis-scaled the wchar_t pointer arithmetic.)

The patch is against the current tree (libcurses/{curses_private.h,screen.c,
getch.c,get_wch.c}); the same code is in base src/lib/libcurses.

diff --git a/libcurses/curses_private.h b/libcurses/curses_private.h
--- a/libcurses/curses_private.h
+++ b/libcurses/curses_private.h
@@ -284,6 +284,7 @@ struct __screen {
  	int notty;
  	int resized;
  	wchar_t *unget_list;
+	bool *unget_key;	/* parallels unget_list: is entry a key code? */
  	int unget_len, unget_pos;
  	int filtered;
  	int checkfd;
@@ -383,7 +384,7 @@ void	__cursesi_putnsp(nschar_t *, const int, const int);
  void	__cursesi_chtype_to_cchar(chtype, cchar_t *);
  #endif /* HAVE_WCHAR */
  int	 __fgetc_resize(FILE *);
-int	 __unget(wint_t);
+int	 __unget(wint_t, bool);
  int	 __mvcur(int, int, int, int, int);
  WINDOW  *__newwin(SCREEN *, int, int, int, int, int, int);
  int	 __nodelay(void);
diff --git a/libcurses/get_wch.c b/libcurses/get_wch.c
--- a/libcurses/get_wch.c
+++ b/libcurses/get_wch.c
@@ -521,7 +521,10 @@ wget_wch(WINDOW *win, wint_t *ch)
  			setcchar(&wc, ws, win->wattr, 0, NULL);
  			wadd_wch(win, &wc);
  		}
-		return KEY_CODE_YES;
+		/* A pushed character is returned as OK; only a pushed key
+		 * code (from ungetch()) is a KEY_CODE_YES. */
+		return _cursesi_screen->unget_key[_cursesi_screen->unget_pos]
+		    ? KEY_CODE_YES : OK;
  	}
  	if (__echoit && !__rawmode) {
  		cbreak();
@@ -635,7 +638,8 @@ wget_wch(WINDOW *win, wint_t *ch)
  int
  unget_wch(const wchar_t c)
  {
-	return __unget((wint_t)c);
+	/* unget_wch() always pushes a character, never a key code. */
+	return __unget((wint_t)c, false);
  }

  /*
diff --git a/libcurses/getch.c b/libcurses/getch.c
--- a/libcurses/getch.c
+++ b/libcurses/getch.c
@@ -924,7 +924,8 @@ wgetch(WINDOW *win)
  int
  ungetch(int c)
  {
-	return __unget((wint_t)c);
+	/* A value in the key-code range is a function key, not a character. */
+	return __unget((wint_t)c, c >= KEY_MIN && c <= KEY_MAX);
  }

  /*
@@ -932,9 +933,10 @@ ungetch(int c)
   *    Do the work for ungetch() and unget_wch();
   */
  int
-__unget(wint_t c)
+__unget(wint_t c, bool iskey)
  {
  	wchar_t	*p;
+	bool	*q;
  	int	len;

  #ifdef DEBUG
@@ -944,25 +946,29 @@ __unget(wint_t c)
  		return ERR;
  	if (_cursesi_screen->unget_pos >= _cursesi_screen->unget_len) {
  		len = _cursesi_screen->unget_len + 32;
+		/* Assign each successful realloc() back at once so the stored
+		 * pointer never dangles if the other realloc() fails. */
  		if ((p = realloc(_cursesi_screen->unget_list,
-		    sizeof(wchar_t) * len)) == NULL) {
-			/* Can't realloc(), so just lose the oldest entry */
+		    sizeof(wchar_t) * len)) != NULL)
+			_cursesi_screen->unget_list = p;
+		if ((q = realloc(_cursesi_screen->unget_key,
+		    sizeof(bool) * len)) != NULL)
+			_cursesi_screen->unget_key = q;
+		if (p == NULL || q == NULL) {
+			/* Can't grow, so just lose the oldest entry. */
  			memmove(_cursesi_screen->unget_list,
-			    _cursesi_screen->unget_list + sizeof(wchar_t),
-			    _cursesi_screen->unget_len - 1);
-			_cursesi_screen->unget_list[_cursesi_screen->unget_len
-			    - 1] = c;
-			_cursesi_screen->unget_pos =
-			    _cursesi_screen->unget_len;
-			return OK;
-		} else {
+			    _cursesi_screen->unget_list + 1,
+			    (_cursesi_screen->unget_len - 1) * sizeof(wchar_t));
+			memmove(_cursesi_screen->unget_key,
+			    _cursesi_screen->unget_key + 1,
+			    (_cursesi_screen->unget_len - 1) * sizeof(bool));
  			_cursesi_screen->unget_pos =
-			    _cursesi_screen->unget_len;
+			    _cursesi_screen->unget_len - 1;
+		} else
  			_cursesi_screen->unget_len = len;
-			_cursesi_screen->unget_list = p;
-		}
  	}
  	_cursesi_screen->unget_list[_cursesi_screen->unget_pos] = c;
+	_cursesi_screen->unget_key[_cursesi_screen->unget_pos] = iskey;
  	_cursesi_screen->unget_pos++;
  	return OK;
  }
diff --git a/libcurses/screen.c b/libcurses/screen.c
--- a/libcurses/screen.c
+++ b/libcurses/screen.c
@@ -168,6 +168,11 @@ newterm(char *type, FILE *outfd, FILE *infd)
  	{
  		goto error_exit;
  	}
+	if ((new_screen->unget_key =
+	    malloc(sizeof(bool) * new_screen->unget_len)) == NULL)
+	{
+		goto error_exit;
+	}
  	new_screen->unget_pos = 0;

  	if (_cursesi_gettmode(new_screen) == ERR)
@@ -233,6 +238,7 @@ newterm(char *type, FILE *outfd, FILE *infd)
    error_exit:
  	__delscreen(new_screen);
  	free(new_screen->unget_list);
+	free(new_screen->unget_key);

  	free(new_screen);
  	return NULL;
@@ -261,6 +267,7 @@ delscreen(SCREEN *screen)

  	free(screen->stdbuf);
  	free(screen->unget_list);
+	free(screen->unget_key);
  	if (_cursesi_screen == screen)
  		_cursesi_screen = NULL;
  	free(screen);

>Unformatted:
 



Home | Main Index | Thread Index | Old Index