>>Geoff Wing <mason@primenet.com.au> typed:
>>:anyone else getting top leaking memory or just me.
>>:% top -s 0
>>
>>Whoops, just noticed that Mason Loring Bliss has already done a send-pr
>>on this.  That's what I get for noticing a problem before I go to sleep
>>and posting about it just after I wake up :-)
>	it looks to me like follows:
>	- cgetstr() calls malloc (as documented in manpage), and returns
>	  pointer to malloc'ed region via 3rd arg
>	  bug: leaks memory when realloc fails
>	- t_getstr does not free the region returned by cgetstr().
>	  not sure what is the intended behavior (should it return a pointer
>	  to malloc'ed region, or not).
>	  bug: it leaks memory when area == NULL.
>	could someone enlighten me on how t_getstr should behave?

	The following patch prevent top to growing.  it seems to be introduced
	by tgoto.c 1.12 -> 1.13 (remove 1024 bytes limit in termcap entry).

	I still have two question:
	- is t_getstr supposed to return pointer to dynamically allocated
	  region?  manpage does not mention it
	- same question for tgetstr.
	if t_gettr/tgetstr returns pointer to dynamically allocated region, I
	see one more memory leak in tputs().

itojun


Index: lib/libc/gen/getcap.c
===================================================================
RCS file: /cvsroot/basesrc/lib/libc/gen/getcap.c,v
retrieving revision 1.32
diff -u -r1.32 getcap.c
--- getcap.c	2000/01/22 22:19:10	1.32
+++ getcap.c	2000/06/02 04:35:43
@@ -249,7 +249,7 @@
 	char *r_end, *rp = NULL, **db_p;	/* pacify gcc */
 	int myfd = 0, eof, foundit, retval;
 	size_t clen;
-	char *record, *cbuf;
+	char *record, *cbuf, *newrecord;
 	int tc_not_resolved;
 	char pbuf[_POSIX_PATH_MAX];
 	
@@ -435,13 +435,15 @@
 
 					pos = rp - record;
 					newsize = r_end - record + BFRAG;
-					record = realloc(record, newsize);
-					if (record == NULL) {
+					newrecord = realloc(record, newsize);
+					if (newrecord == NULL) {
+						free(record);
 						if (myfd)
 							(void)close(fd);
 						errno = ENOMEM;
 						return (-2);
 					}
+					record = newrecord;
 					r_end = record + newsize;
 					rp = record + pos;
 				}
@@ -577,14 +579,16 @@
 				newsize = r_end - record + diff + BFRAG;
 				tcpos = tcstart - record;
 				tcposend = tcend - record;
-				record = realloc(record, newsize);
-				if (record == NULL) {
+				newrecord = realloc(record, newsize);
+				if (newrecord == NULL) {
+					free(record);
 					if (myfd)
 						(void)close(fd);
 					free(icap);
 					errno = ENOMEM;
 					return (-2);
 				}
+				record = newrecord;
 				r_end = record + newsize;
 				rp = record + pos;
 				tcstart = record + tcpos;
@@ -615,12 +619,15 @@
 	if (myfd)
 		(void)close(fd);
 	*len = rp - record - 1;	/* don't count NUL */
-	if (r_end > rp)
-		if ((record = 
+	if (r_end > rp) {
+		if ((newrecord = 
 		     realloc(record, (size_t)(rp - record))) == NULL) {
+			free(record);
 			errno = ENOMEM;
 			return (-2);
 		}
+		record = newrecord;
+	}
 		
 	*cap = record;
 	if (tc_not_resolved)
@@ -887,7 +894,7 @@
 	const char *bp;
 	char *mp;
 	int len;
-	char *mem;
+	char *mem, *newmem;
 
 	_DIAGASSERT(buf != NULL);
 	_DIAGASSERT(cap != NULL);
@@ -978,8 +985,11 @@
 		if (m_room == 0) {
 			size_t size = mp - mem;
 
-			if ((mem = realloc(mem, size + SFRAG)) == NULL)
+			if ((newmem = realloc(mem, size + SFRAG)) == NULL) {
+				free(mem);
 				return (-2);
+			}
+			mem = newmem;
 			m_room = SFRAG;
 			mp = mem + size;
 		}
@@ -991,9 +1001,13 @@
 	/*
 	 * Give back any extra memory and return value and success.
 	 */
-	if (m_room != 0)
-		if ((mem = realloc(mem, (size_t)(mp - mem))) == NULL)
+	if (m_room != 0) {
+		if ((newmem = realloc(mem, (size_t)(mp - mem))) == NULL) {
+			free(mem);
 			return (-2);
+		}
+		mem = newmem;
+	}
 	*str = mem;
 	return (len);
 }
@@ -1018,7 +1032,7 @@
 	const char *bp;
 	char *mp;
 	int len;
-	char *mem;
+	char *mem, *newmem;
 
 	_DIAGASSERT(buf != NULL);
 	_DIAGASSERT(cap != NULL);
@@ -1058,8 +1072,11 @@
 		if (m_room == 0) {
 			size_t size = mp - mem;
 
-			if ((mem = realloc(mem, size + SFRAG)) == NULL)
+			if ((newmem = realloc(mem, size + SFRAG)) == NULL) {
+				free(mem);
 				return (-2);
+			}
+			mem = newmem;
 			m_room = SFRAG;
 			mp = mem + size;
 		}
@@ -1071,9 +1088,13 @@
 	/*
 	 * Give back any extra memory and return value and success.
 	 */
-	if (m_room != 0)
-		if ((mem = realloc(mem, (size_t)(mp - mem))) == NULL)
+	if (m_room != 0) {
+		if ((newmem = realloc(mem, (size_t)(mp - mem))) == NULL) {
+			free(mem);
 			return (-2);
+		}
+		mem = newmem;
+	}
 	*str = mem;
 	return (len);
 }
Index: lib/libterm/termcap.c
===================================================================
RCS file: /cvsroot/basesrc/lib/libterm/termcap.c,v
retrieving revision 1.31
diff -u -r1.31 termcap.c
--- termcap.c	2000/05/28 09:58:15	1.31
+++ termcap.c	2000/06/02 04:35:44
@@ -345,6 +345,7 @@
 		 */
 		if (limit != NULL && (*limit < i)) {
 			errno = E2BIG;
+			free(s);
 			return NULL;
 		}
   	
@@ -356,6 +357,7 @@
 	} else {
 		_DIAGASSERT(limit != NULL);
 		*limit = i;
+		free(s);
 		return NULL;
 	}
 }
Index: lib/libterm/tgoto.c
===================================================================
RCS file: /cvsroot/basesrc/lib/libterm/tgoto.c,v
retrieving revision 1.15
diff -u -r1.15 tgoto.c
--- tgoto.c	1999/09/16 11:45:49	1.15
+++ tgoto.c	2000/06/02 04:35:44
@@ -45,6 +45,7 @@
 #include <errno.h>
 #include <string.h>
 #include <termcap.h>
+#include <stdlib.h>
 
 #define	CTRL(c)	((c) & 037)
 
@@ -129,8 +130,14 @@
 	if (cp == 0) {
                 errno = EINVAL;
 toohard:
-                UP = old_up;
-                BC = old_bc;
+		if (UP != old_up) {
+			free(UP);
+			UP = old_up;
+		}
+		if (BC != old_bc) {
+			free(BC);
+			BC = old_bc;
+		}
 		return -1;
 	}
 	added[0] = '\0';
@@ -292,7 +299,13 @@
         }
         
         (void)strcpy(dp, added);
-        UP = old_up;
-        BC = old_bc;
+	if (UP != old_up) {
+		free(UP);
+		UP = old_up;
+	}
+	if (BC != old_bc) {
+		free(BC);
+		BC = old_bc;
+	}
 	return 0;
 }