NetBSD Problem Report #56298

From www@netbsd.org  Sun Jul  4 07:33:54 2021
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_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 C0E141A923B
	for <gnats-bugs@gnats.NetBSD.org>; Sun,  4 Jul 2021 07:33:54 +0000 (UTC)
Message-Id: <20210704065351.67A121A923B@mollari.NetBSD.org>
Date: Sun,  4 Jul 2021 06:53:51 +0000 (UTC)
From: mforney@mforney.org
Reply-To: mforney@mforney.org
To: gnats-bugs@NetBSD.org
Subject: libcurses: assertion failure when $TERM lacks certain terminfo capabilities
X-Send-Pr-Version: www-1.0

>Number:         56298
>Category:       lib
>Synopsis:       libcurses: assertion failure when $TERM lacks certain terminfo capabilities
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    blymn
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jul 04 07:35:00 +0000 2021
>Closed-Date:    Tue Apr 12 22:13:12 +0000 2022
>Last-Modified:  Tue Apr 12 22:13:12 +0000 2022
>Originator:     Michael Forney
>Release:        
>Organization:
>Environment:
>Description:
If libcurses is built with -D_DIAGNOSTIC, assertion failures are triggered in the ti_puts family of functions when the terminal lacks certain capabilities since they are called with str==NULL. In particular, this happens with TERM=linux.

Here are a couple of problematic codepaths:

libcurses/tty.c:__startwin:577: ti_puts(screen->term, t_enter_ca_mode(screen->term), 0, __cputchar_args, (void *) screen->outfd);
libterminfo/tputs:ti_puts:138: _DIAGASSERT(str != NULL);

If term has no smcup, t_enter_ca_mode returns NULL and the assert fails.

libcurses/keypad.c:keypad:52: tputs(keypad_xmit, 0, __cputchar);
libterminfo/tputs.c:tputs:167: _DIAGASSERT(str != NULL);

If term has no smkx, keypad_xmit evaluates to NULL and the assert fails.

The _ti_puts function is the base implementation for all these functions, and it handles str==NULL just fine by returning OK immediately. So it seems that these _DIAGASSERTs are unnecessary and should be removed.
>How-To-Repeat:
Build libcurses with -D_DIAGNOSTIC, then run a program using libcurses that calls newterm() and keypad() (such as the vis text editor).
>Fix:
I think the problematic assertions should be removed:

diff --git a/lib/libterminfo/tputs.c b/lib/libterminfo/tputs.c
index f1e15bbc613c..aa2118da6dc9 100644
--- a/lib/libterminfo/tputs.c
+++ b/lib/libterminfo/tputs.c
@@ -135,7 +135,6 @@ ti_puts(const TERMINAL *term, const char *str, int affcnt,
 	char pc;

 	_DIAGASSERT(term != NULL);
-	_DIAGASSERT(str != NULL);
 	_DIAGASSERT(outc != NULL);

 	dodelay = (str == t_bell(term) ||
@@ -155,7 +154,6 @@ ti_putp(const TERMINAL *term, const char *str)
 {

 	_DIAGASSERT(term != NULL);
-	_DIAGASSERT(str != NULL);
 	return ti_puts(term, str, 1,
 	    (int (*)(int, void *))(void *)putchar, NULL);
 }
@@ -164,7 +162,6 @@ int
 tputs(const char *str, int affcnt, int (*outc)(int))
 {

-	_DIAGASSERT(str != NULL);
 	_DIAGASSERT(outc != NULL);
 	return _ti_puts(1, ospeed, PC, str, affcnt,
 	    (int (*)(int, void *))(void *)outc, NULL);
@@ -174,6 +171,5 @@ int
 putp(const char *str)
 {

-	_DIAGASSERT(str != NULL);
 	return tputs(str, 1, putchar);
 }

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: lib-bug-people->blymn
Responsible-Changed-By: blymn@NetBSD.org
Responsible-Changed-When: Wed, 15 Dec 2021 21:08:48 +0000
Responsible-Changed-Why:
I will claim this bug.


State-Changed-From-To: open->feedback
State-Changed-By: blymn@NetBSD.org
State-Changed-When: Wed, 15 Dec 2021 21:08:48 +0000
State-Changed-Why:
Fix has been committed, requesting feedback.


From: "Brett Lymn" <blymn@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56298 CVS commit: src/lib/libterminfo
Date: Wed, 15 Dec 2021 21:07:13 +0000

 Module Name:	src
 Committed By:	blymn
 Date:		Wed Dec 15 21:07:12 UTC 2021

 Modified Files:
 	src/lib/libterminfo: tputs.c

 Log Message:
 Fix for PR lib/56298

 Remove the DIAGASSERT for str being NULL in the puts/putp functions,
 add protection so that the functions just return OK if str is NULL.
 This prevents the assert firing when libcurses passes through a NULL
 due to an undefined terminfo entry.


 To generate a diff of this commit:
 cvs rdiff -u -r1.5 -r1.6 src/lib/libterminfo/tputs.c

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

State-Changed-From-To: feedback->closed
State-Changed-By: blymn@NetBSD.org
State-Changed-When: Tue, 12 Apr 2022 22:13:12 +0000
State-Changed-Why:
This was fixed, no response to request for feedback.


>Unformatted:

Home	PR Database Search