Subject: lib/14625: programs that use termcap make free() noise
To: None <gnats-bugs@gnats.netbsd.org>
From: g r <gr@grappa.eclipsed.net>
List: netbsd-bugs
Date: 11/18/2001 13:31:43
>Number:         14625
>Category:       lib
>Synopsis:       programs that use termcap make free() noise
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Nov 18 10:32:01 PST 2001
>Closed-Date:
>Last-Modified:
>Originator:     gabriel rosenkoetter
>Release:        NetBSD 1.5Y 20011118
>Organization:
	
>Environment:
System: NetBSD grappa 1.5Y NetBSD 1.5Y (GRAPPA) #1: Wed Nov 14 21:37:52 EST 2001 gr@grappa:/usr/src/sys/arch/i386/compile/GRAPPA i386
Architecture: i386
Machine: i386
>Description:
In revision 1.31 of src/lib/libcurses/getch.c, a change was made to
work around the fact that t_getstr() from src/lib/libterm/termcap.c
some times returned a non-NULL value which would then not be
free()ed properly leading to a memory leak.

Revisions 1.39 and 1.40 to src/lib/libterm/termcap.c seem to have
fixed this bug at its source, making the workaround unnecessary.
(BUT!!! I don't understand the circumstances necessary to provoke
this memory leak bug, so I hope that someone who does will double
check this before committing it!)

Revision 1.34 of src/lib/libcurses/getch.c removed the (superfluous)
free() in getch.c without removing the framework to check the return
value. As near as I can tell, this means that the memory which
everyone's trying to free() is referenced externally (in getch.c)
when someone in the right place (termcap.c) tries to free() it,
which leads to vi(1) and robots(6) (along with anything else that
uses t_getstr()) spewing:

vi in free(): warning: junk pointer, too high to make sense.

You may want to see:

  http://mail-index.netbsd.org/current-users/2001/11/02/0046.html and

the ensuing thread.
>How-To-Repeat:
Use vi(1) or robots(6) on a -current, post-20011101 system.
>Fix:
This patch removes the rest of the 1.31 scaffolding to work around
the non-free()ing in t_getstr() by checking it locally. (This avoids
storing the return value of t_getstr() in a local variable, which,
as near as I can tell, is what's causing free() to complain.)

This, along with revision 1.34 (already committed to the tree) is
effectively a reversion of 1.31 to 1.30. If there's some fancy cvs
method to revert 1.34, then revert 1.31 (leaving 1.32 and 33 in
tact), that may be deemed more appropriate.

Index: lib/libcurses/getch.c
===================================================================
RCS file: /cvsroot/basesrc/lib/libcurses/getch.c,v
retrieving revision 1.34
diff -u -r1.34 getch.c
--- getch.c	2001/11/01 16:06:59	1.34
+++ getch.c	2001/11/15 03:04:42
@@ -379,7 +379,6 @@
 	size_t limit;
 	key_entry_t *tmp_key;
 	keymap_t *current;
-	char *cp;
 #ifdef DEBUG
 	int k;
 #endif
@@ -397,8 +396,7 @@
 	for (i = 0; i < num_tcs; i++) {
 		p = entry;
 		limit = 1023;
-		cp = t_getstr(_cursesi_genbuf, tc[i].name, &p, &limit);
-		if (cp != NULL) {
+		if (t_getstr(_cursesi_genbuf,  tc[i].name, &p, &limit) != NULL) {
 			current = base_keymap;	/* always start with
 						 * base keymap. */
 			length = (int) strlen(entry);
>Release-Note:
>Audit-Trail:
>Unformatted: