Re: lib/56173: libcurses: reset_prog_mode() after reset_shell_mode() overwrites original TTY state

[Christos Zoulas <christos%zoulas.com@localhost>]

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

From: Christos Zoulas <christos%zoulas.com@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: lib-bug-people%netbsd.org@localhost,
 gnats-admin%netbsd.org@localhost,
 netbsd-bugs%netbsd.org@localhost
Subject: Re: lib/56173: libcurses: reset_prog_mode() after reset_shell_mode()
 overwrites original TTY state
Date: Mon, 31 May 2021 13:07:09 -0400

 --Apple-Mail=_6F304CAD-4C8E-4B9A-8419-26EA703F25C3
 Content-Transfer-Encoding: quoted-printable
 Content-Type: text/plain;
 	charset=us-ascii
 
 This program behaves very differently under ncurses and curses:
 
 include <stdio.h>
 #include <curses.h>
 #include <stdlib.h>
 
 int
 main(void)
 {
         printf("\r\ninitially\n");
         system("stty -a");
         initscr();
         printf("\r\nafter initscr\n");
         system("stty -a");
         reset_shell_mode();
         printf("\r\nafter reset_shell_mode\n");
         system("stty -a");
         reset_prog_mode();
         printf("\r\nafter reset_prog_mode\n");
         system("stty -a");
         endwin();
         printf("\r\nafter endwin\n");
         system("stty -a");
         return 0;
 }
 
 The question is who is right?
 
 christos
 
 > On May 15, 2021, at 12:15 AM, mforney%mforney.org@localhost wrote:
 >=20
 >> Number:         56173
 >> Category:       lib
 >> Synopsis:       libcurses: reset_prog_mode() after reset_shell_mode() =
 overwrites original TTY state
 >> Confidential:   no
 >> Severity:       serious
 >> Priority:       medium
 >> Responsible:    lib-bug-people
 >> State:          open
 >> Class:          sw-bug
 >> Submitter-Id:   net
 >> Arrival-Date:   Sat May 15 04:15:00 +0000 2021
 >> Originator:     Michael Forney
 >> Release:        trunk
 >> Organization:
 >> Environment:
 >> Description:
 > If a curses program calls reset_shell_mode() and then =
 reset_prog_mode() to temporarily switch to shell mode and then back to =
 curses mode, the original TTY settings are overwritten. This results in =
 incorrect TTY settings after the application exits.
 >=20
 > This pattern is used in the text editor vis when you read output from =
 a shell command.
 >=20
 > The problem seems to be that reset_shell_mode() calls __stopwin() to =
 save the curses mode settings and restore the shell mode settings, but =
 reset_prog_mode() just restores the saved curses mode settings, leaving =
 the screen in the "endwin" state. Then, when the application performs a =
 refresh(), __restartwin() is called which saves the current (already =
 restored) settings as the original settings, and then restores the =
 curses mode settings again. At this point, the original settings have =
 been overwritten with the curses mode settings, which persist even after =
 the application calls endwin() and exits.
 >=20
 > In other words, the following sequence of events occurs:
 >=20
 > 	reset_shell_mode()
 > 		__stopwin()
 > 			saved settings =3D current settings
 > 			current settings =3D original settings
 > 			endwin =3D 1
 > 	reset_prog_mode()
 > 		current settings =3D saved settings
 > 	refresh()
 > 		__restartwin()
 > 			original settings =3D current settings
 > 			current settings =3D saved settings
 >=20
 > At this point we have original settings =3D=3D current settings =3D=3D =
 curses mode settings. The original settings have been overwritten.
 >=20
 > It appears that this behavior dates back to this commit from 2003:
 > =
 https://github.com/NetBSD/src/commit/880234af7eb36a86d6adfa775327ccec592d5=
 19d#diff-3825aa201b54dbfe36d66fe3f650737571d39e3459b936691d1c955f37871d2dL=
 199-R204
 >=20
 > Previously, reset_shell_mode() and reset_prog_mode() were symmetrical, =
 calling __endwin() and __restartwin() respectively. After the change, =
 reset_shell_mode() still calls __endwin(), but reset_prog_mode() just =
 changes the TTY settings leaving the screen in the endwin state.
 >> How-To-Repeat:
 > 1. Build vis (https://github.com/martanne/vis) against NetBSD =
 libcurses, then run it.
 > 2. Type ":<echo hello" to read the output of a command into the =
 buffer.
 > 3. Exit vis with ":q!".
 > 4. Terminal settings are now messed up.
 >> Fix:
 > I don't quite understand what problem the commit referenced above was =
 attempting to fix, but if I revert the highlighted hunk, the problem is =
 fixed.
 >=20
 > diff --git a/lib/libcurses/tstp.c b/lib/libcurses/tstp.c
 > index f0baa999120b..ef54143a4a54 100644
 > --- a/lib/libcurses/tstp.c
 > +++ b/lib/libcurses/tstp.c
 > @@ -348,8 +348,8 @@ int
 > reset_prog_mode(void)
 > {
 >=20
 > -	return tcsetattr(fileno(_cursesi_screen->infd), TCSASOFT | =
 TCSADRAIN,
 > -			 &_cursesi_screen->save_termios) ? ERR : OK;
 > +	__restartwin();
 > +	return OK;
 > }
 >=20
 > int
 
 
 --Apple-Mail=_6F304CAD-4C8E-4B9A-8419-26EA703F25C3
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
 	filename=signature.asc
 Content-Type: application/pgp-signature;
 	name=signature.asc
 Content-Description: Message signed with OpenPGP
 
 -----BEGIN PGP SIGNATURE-----
 Comment: GPGTools - http://gpgtools.org
 
 iF0EARECAB0WIQS+BJlbqPkO0MDBdsRxESqxbLM7OgUCYLUXvQAKCRBxESqxbLM7
 OgDzAJ4ulhf4OsPZOcIHyjSKH7O1HGY2bACg4cqhYJXyUIGVEA8bACdHAZpFM70=
 =od53
 -----END PGP SIGNATURE-----
 
 --Apple-Mail=_6F304CAD-4C8E-4B9A-8419-26EA703F25C3--