NetBSD Problem Report #53615

From www@NetBSD.org  Tue Sep 18 12:16:42 2018
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id D6F9A7A1B0
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 18 Sep 2018 12:16:41 +0000 (UTC)
Message-Id: <20180918121640.E50947A261@mollari.NetBSD.org>
Date: Tue, 18 Sep 2018 12:16:40 +0000 (UTC)
From: n54@gmx.com
Reply-To: n54@gmx.com
To: gnats-bugs@NetBSD.org
Subject: curses(3) KEY_RESIZE does not work
X-Send-Pr-Version: www-1.0

>Number:         53615
>Category:       lib
>Synopsis:       curses(3) KEY_RESIZE does not work
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Sep 18 12:20:00 +0000 2018
>Closed-Date:    Thu Sep 27 15:04:52 +0000 2018
>Last-Modified:  Thu Sep 27 15:04:52 +0000 2018
>Originator:     Kamil Rytarowski
>Release:        NetBSD 8.99.24 amd64
>Organization:
TNF
>Environment:
NetBSD rugged 8.99.24 NetBSD 8.99.24 (GENERIC) #8: Fri Aug 31 21:55:37 CEST 2018  root@chieftec:/public/netbsd-root/sys/arch/amd64/compile/GENERIC amd64
>Description:
getch(3) documents:

If the terminal is resized, getch() will return KEY_RESIZE,
regardless of the setting of keypad().

However with and without keypad(3) option, getch(3) returns -1 (ERR).

The same code works well with ncurses.
>How-To-Repeat:
#include <stdio.h>
#include <curses.h>

int
main(int argc, char **argv)
{
	initscr();
	int w = getch();
	endwin();

	printf("w=%d\n", w);

	return 0;
}

$ gcc -lcurses key.c
$ ./a.out
# resize terminal
# see report -1 (ERR)
>Fix:
N/A

>Release-Note:

>Audit-Trail:
From: Rin Okuyama <rokuyama@rk.phys.keio.ac.jp>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/53615: curses(3) KEY_RESIZE does not work
Date: Wed, 19 Sep 2018 00:32:40 +0900

 This is because it does not check whether windows is resized or not
 when fgetc() fails with EINTR. This regression was introduced in
 getch.c r1.50:

 http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libcurses/getch.c#rev1.50

 This patch fixes your problem, and I hope that it breaks nothing...

 ----
 Index: lib/libcurses/get_wch.c
 ===================================================================
 RCS file: /home/netbsd/src/lib/libcurses/get_wch.c,v
 retrieving revision 1.14
 diff -p -u -r1.14 get_wch.c
 --- lib/libcurses/get_wch.c	31 Jan 2017 09:17:53 -0000	1.14
 +++ lib/libcurses/get_wch.c	18 Sep 2018 14:35:59 -0000
 @@ -39,6 +39,7 @@
   __RCSID("$NetBSD: get_wch.c,v 1.14 2017/01/31 09:17:53 roy Exp $");
   #endif						  /* not lint */

 +#include <errno.h>
   #include <string.h>
   #include <stdlib.h>
   #include <unistd.h>
 @@ -596,7 +597,11 @@ wget_wch(WINDOW *win, wint_t *ch)

   		if (ferror(infd)) {
   			clearerr(infd);
 -			return ERR;
 +			if (errno == EINTR && _cursesi_screen->resized) {
 +				*ch = KEY_RESIZE;
 +				return KEY_CODE_YES;
 +			} else
 +				return ERR;
   		} else {
   			ret = c;
   			inp = c;
 Index: lib/libcurses/getch.c
 ===================================================================
 RCS file: /home/netbsd/src/lib/libcurses/getch.c,v
 retrieving revision 1.65
 diff -p -u -r1.65 getch.c
 --- lib/libcurses/getch.c	31 Jan 2017 09:17:53 -0000	1.65
 +++ lib/libcurses/getch.c	18 Sep 2018 14:31:28 -0000
 @@ -38,6 +38,7 @@ __RCSID("$NetBSD: getch.c,v 1.65 2017/01
   #endif
   #endif					/* not lint */

 +#include <errno.h>
   #include <string.h>
   #include <stdlib.h>
   #include <unistd.h>
 @@ -889,7 +890,11 @@ wgetch(WINDOW *win)

   		if (ferror(infd)) {
   			clearerr(infd);
 -			inp = ERR;
 +			if (errno == EINTR && _cursesi_screen->resized) {
 +				_cursesi_screen->resized = 0;
 +				inp = KEY_RESIZE;
 +			} else
 +				inp = ERR;
   		} else {
   			inp = c;
   		}
 ----

 For SIGWINCH handler, I found inappropriate NULL check for sa_handler.
 It should be compared to all of SIG_* in <sys/signal.h>, as in a
 similar manner to jmcneill's fix for nvi:

 http://cvsweb.netbsd.org/bsdweb.cgi/src/external/bsd/nvi/dist/cl/cl_main.c#rev1.9

 Also, I don't understand why _cursei_screen->resized is not updated
 when a previous handler exists. How do you think of this patch?

 ----
 Index: lib/libcurses/tstp.c
 ===================================================================
 RCS file: /home/netbsd/src/lib/libcurses/tstp.c,v
 retrieving revision 1.42
 diff -p -u -r1.42 tstp.c
 --- lib/libcurses/tstp.c	6 Jan 2017 13:53:18 -0000	1.42
 +++ lib/libcurses/tstp.c	18 Sep 2018 15:19:16 -0000
 @@ -148,12 +148,15 @@ __winch_signal_handler(/*ARGSUSED*/int s
   	}
   	/*
   	 * If there was a previous handler, call that,
 -	 * otherwise tell getch() to send KEY_RESIZE.
 +	 * then tell getch() to send KEY_RESIZE.
   	 */
 -	if (owsa.sa_handler !=  NULL)
 +	if (owsa.sa_handler != SIG_DFL &&
 +	    owsa.sa_handler != SIG_IGN &&
 +	    owsa.sa_handler != SIG_ERR &&
 +	    owsa.sa_handler != SIG_HOLD)
   		owsa.sa_handler(signo);
 -	else
 -		_cursesi_screen->resized = 1;
 +
 +	_cursesi_screen->resized = 1;
   }

   /*
 ----

 Thanks
 rin

From: Roy Marples <roy@marples.name>
To: gnats-bugs@NetBSD.org
Cc: Rin Okuyama <rokuyama@rk.phys.keio.ac.jp>
Subject: Re: lib/53615: curses(3) KEY_RESIZE does not work
Date: Tue, 18 Sep 2018 21:20:42 +0100

 The above patch looks fine with one caveat - we should not set 
 _cursesi_screen->resized = 1 if we already have a signal handler for 
 SIGWINCH.

 Roy

From: Rin Okuyama <rokuyama@rk.phys.keio.ac.jp>
To: Roy Marples <roy@marples.name>, gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/53615: curses(3) KEY_RESIZE does not work
Date: Wed, 19 Sep 2018 06:12:43 +0900

 Thank you very much for your kind review! But why we should not set
 _cursesi_screen->resized when we already have SIGWINCH handler?

 rin

From: Roy Marples <roy@marples.name>
To: Rin Okuyama <rokuyama@rk.phys.keio.ac.jp>, gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/53615: curses(3) KEY_RESIZE does not work
Date: Tue, 18 Sep 2018 22:55:04 +0100

 On 18/09/2018 22:12, Rin Okuyama wrote:
 > Thank you very much for your kind review! But why we should not set
 > _cursesi_screen->resized when we already have SIGWINCH handler?

 You're either going to use KEY_RESIZE or SIGWINCH in the application.
 What use case do you have for handling both?

 Roy

From: Rin Okuyama <rokuyama@rk.phys.keio.ac.jp>
To: Roy Marples <roy@marples.name>, gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/53615: curses(3) KEY_RESIZE does not work
Date: Wed, 19 Sep 2018 07:00:57 +0900

 On 2018/09/19 6:55, Roy Marples wrote:
 > You're either going to use KEY_RESIZE or SIGWINCH in the application.
 > What use case do you have for handling both?

 Ah, I understand the intention of the original code. Thank you for
 explanation! I will commit the rest parts soon.

 rin

From: Roy Marples <roy@marples.name>
To: Rin Okuyama <rokuyama@rk.phys.keio.ac.jp>, gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/53615: curses(3) KEY_RESIZE does not work
Date: Tue, 18 Sep 2018 23:12:56 +0100

 On 18/09/2018 23:00, Rin Okuyama wrote:
 > On 2018/09/19 6:55, Roy Marples wrote:
 >> You're either going to use KEY_RESIZE or SIGWINCH in the application.
 >> What use case do you have for handling both?
 > 
 > Ah, I understand the intention of the original code. Thank you for
 > explanation! I will commit the rest parts soon.

 When you do, ensure to set _cursesi_screen->resized to 0 when returning 
 KEY_RESIZE in lib/libcurses/get_wch.c

 Your current patch omits that as well.

 Roy

From: Rin Okuyama <rokuyama@rk.phys.keio.ac.jp>
To: Roy Marples <roy@marples.name>, gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/53615: curses(3) KEY_RESIZE does not work
Date: Wed, 19 Sep 2018 07:35:12 +0900

 On 2018/09/19 7:12, Roy Marples wrote:
 > When you do, ensure to set _cursesi_screen->resized to 0 when returning KEY_RESIZE in lib/libcurses/get_wch.c
 > 
 > Your current patch omits that as well.

 Oops, I forgot it. Thank you for finding it out.

 rin

From: "Rin Okuyama" <rin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53615 CVS commit: src/lib/libcurses
Date: Tue, 18 Sep 2018 22:46:18 +0000

 Module Name:	src
 Committed By:	rin
 Date:		Tue Sep 18 22:46:18 UTC 2018

 Modified Files:
 	src/lib/libcurses: get_wch.c getch.c

 Log Message:
 PR lib/53615

 getch() and get_wch() should return KEY_RESIZE when interrupted by SIGWINCH.

 OK roy


 To generate a diff of this commit:
 cvs rdiff -u -r1.14 -r1.15 src/lib/libcurses/get_wch.c
 cvs rdiff -u -r1.65 -r1.66 src/lib/libcurses/getch.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Rin Okuyama" <rin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53615 CVS commit: src/lib/libcurses
Date: Tue, 18 Sep 2018 22:51:00 +0000

 Module Name:	src
 Committed By:	rin
 Date:		Tue Sep 18 22:51:00 UTC 2018

 Modified Files:
 	src/lib/libcurses: tstp.c

 Log Message:
 PR lib/53615

 Before invoking a previous signal handler, make sure it is not SIG_*.
 Fix potential crash with SIGWINCH.

 OK roy


 To generate a diff of this commit:
 cvs rdiff -u -r1.42 -r1.43 src/lib/libcurses/tstp.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Kamil Rytarowski <n54@gmx.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: PR/53615 CVS commit: src/lib/libcurses
Date: Wed, 26 Sep 2018 15:26:57 +0200

 This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
 --oqb9aGdEbjpycYJRwGmu2BkkZCqMtnvcT
 Content-Type: multipart/mixed; boundary="wCRonlWLaRCJyEsL7Xw3HCbyX0a0Oyxyg";
  protected-headers="v1"
 From: Kamil Rytarowski <n54@gmx.com>
 To: gnats-bugs@NetBSD.org
 Message-ID: <50e91d32-b246-2ce3-192e-400ce3fa9f75@gmx.com>
 Subject: Re: PR/53615 CVS commit: src/lib/libcurses
 References: <pr-lib-53615@gnats.netbsd.org>
  <20180918121640.E50947A261@mollari.NetBSD.org>
  <20180918225001.C64EB7A212@mollari.NetBSD.org>
 In-Reply-To: <20180918225001.C64EB7A212@mollari.NetBSD.org>

 --wCRonlWLaRCJyEsL7Xw3HCbyX0a0Oyxyg
 Content-Type: text/plain; charset=utf-8
 Content-Language: en-US
 Content-Transfer-Encoding: quoted-printable

 Thank you for a patch for curses(3), it made the things better.

 KEY_RESIZE is still broken with enabled keypad(3).

 A reproducer:

 #include <curses.h>
 #include <stdio.h>

 int
 main(int argc, char **argv)
 {

 	initscr();

 	keypad(stdscr, TRUE);

 	int ch =3D wgetch(stdscr);

 	endwin();

 	printf("ch =3D %d\n", ch);

 	return 0;
 }


 --wCRonlWLaRCJyEsL7Xw3HCbyX0a0Oyxyg--

 --oqb9aGdEbjpycYJRwGmu2BkkZCqMtnvcT
 Content-Type: application/pgp-signature; name="signature.asc"
 Content-Description: OpenPGP digital signature
 Content-Disposition: attachment; filename="signature.asc"

 -----BEGIN PGP SIGNATURE-----

 iQJABAEBCAAqFiEELaxVpweEzw+lMDwuS7MI6bAudmwFAluriSEMHG41NEBnbXgu
 Y29tAAoJEEuzCOmwLnZsPiEP/0HqdA27Ct+Yqe+agXuWCxeCPZ+HIF+7FFpZmwOl
 0MDlWttZxjNJSsXruaZNe6vVBr2kCw7y9W16w4DgvVYNvnyCUD9ptB11EGKAs/NA
 LRyI9qm+DA9U/5MwXKEoAMDVXoTmvjmIhBKjFS++Kere4xgoA98w/uDO85Vnjr0+
 vExqV+hLSxjwt84XXQu+WYc4+zETQp2VVKkWHTWmCz1cykA8tnTtXfMv7hHaaDyf
 fAlnsOb5qErAkbarX8vjYx8XFdQaq1rNo7ZLlsIzPwmCTfjyI1qB+nPtLqL8vkSC
 Ee0olfTk/mtRdzSu4Nfotilbe0/tqW9s9hBbfTvrTrfSrzZaclj41ndjrZHikfd1
 NZsU9e0s04LScb6rO9GFfCNPp8VP78/27xFANDgRb4fnIH7CkbwvwuqcDGr72u+v
 6bLLHs7g/SKf0zBuEPDJhiwPH7ZZNg7uTqnm3YDGnsJG2Ep3oa84+uCKks3h8uNB
 sE1c5Q4s5daUGnkr9cz8CO2+4bq+bfrogtJqNAdEA0NW9Fyv9MXnBnPad/5iz4Vi
 BKC86hXm7dc5eigxFSE38FIDeB4KltRcZ7aLblvuhW9kS+J6E0NUo8yJysXZrwJ1
 UGPtCyrNu+57hQ2U33kima16mzVI8X93nuN8tYuuw0IrckdCVhkRVcDNoLU6aLXc
 Y6J/
 =e9Dv
 -----END PGP SIGNATURE-----

 --oqb9aGdEbjpycYJRwGmu2BkkZCqMtnvcT--

From: "Kamil Rytarowski" <kamil@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53615 CVS commit: src/lib/libcurses
Date: Wed, 26 Sep 2018 14:42:22 +0000

 Module Name:	src
 Committed By:	kamil
 Date:		Wed Sep 26 14:42:22 UTC 2018

 Modified Files:
 	src/lib/libcurses: get_wch.c getch.c

 Log Message:
 Correct detecting of terminal resize in curses(3) with keypad(,TRUE)

 A previous change fixed only keypad(,FALSE) scenarios.

 Handle catching terminal resize in INKEY_NORM and INKEY_ASSEMBLING (in the
 middle of assembling a key code from passed codes) as both accept keys with
 fgetc(3) and both can be in theory interrupted with a resize.

 PR lib/53615


 To generate a diff of this commit:
 cvs rdiff -u -r1.15 -r1.16 src/lib/libcurses/get_wch.c
 cvs rdiff -u -r1.66 -r1.67 src/lib/libcurses/getch.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->pending-pullups
State-Changed-By: kamil@NetBSD.org
State-Changed-When: Wed, 26 Sep 2018 18:09:50 +0200
State-Changed-Why:
pullup-8 #1039


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53615 CVS commit: [netbsd-8] src/lib/libcurses
Date: Thu, 27 Sep 2018 14:59:28 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Thu Sep 27 14:59:28 UTC 2018

 Modified Files:
 	src/lib/libcurses [netbsd-8]: get_wch.c getch.c tstp.c

 Log Message:
 Pull up following revision(s) (requested by kamil in ticket #1039):

 	lib/libcurses/getch.c: revision 1.66
 	lib/libcurses/getch.c: revision 1.67
 	lib/libcurses/tstp.c: revision 1.43
 	lib/libcurses/get_wch.c: revision 1.15
 	lib/libcurses/get_wch.c: revision 1.16

 PR lib/53615
 getch() and get_wch() should return KEY_RESIZE when interrupted by SIGWIN=
 CH.

 OK roy

  -

 PR lib/53615
 Before invoking a previous signal handler, make sure it is not SIG_*.
 Fix potential crash with SIGWINCH.

 OK roy

  -

 Correct detecting of terminal resize in curses(3) with keypad(,TRUE)
 A previous change fixed only keypad(,FALSE) scenarios.

  -

 Handle catching terminal resize in INKEY_NORM and INKEY_ASSEMBLING (in the
 middle of assembling a key code from passed codes) as both accept keys with
 fgetc(3) and both can be in theory interrupted with a resize.

 PR lib/53615


 To generate a diff of this commit:
 cvs rdiff -u -r1.14 -r1.14.4.1 src/lib/libcurses/get_wch.c
 cvs rdiff -u -r1.65 -r1.65.4.1 src/lib/libcurses/getch.c
 cvs rdiff -u -r1.42 -r1.42.6.1 src/lib/libcurses/tstp.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: kamil@NetBSD.org
State-Changed-When: Thu, 27 Sep 2018 17:04:52 +0200
State-Changed-Why:
Merged with -8 pullup-8 #1039
lib/libcurses/get_wch.c				1.15,1.16
lib/libcurses/getch.c				1.66,1.67
lib/libcurses/tstp.c				1.43


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.43 2018/01/16 07:36:43 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.