>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?

itojun


(gdb) bt
#0  0x8070a3d in malloc ()
#1  0x8055fe7 in cgetstr ()
#2  0x804d0ec in t_getstr ()
#3  0x804c832 in t_goto ()
#4  0x804b181 in Move_to (x=49, y=7)
    at /home/itojun/NetBSD/src/usr.bin/top/screen.c:508
#5  0x804aa6d in line_update (old=0x8096080 "00  0.07%  0.05% top", 
    new=0x8083612 "7  0.00%  0.00% sshd", start=0, line=7)
    at /home/itojun/NetBSD/src/usr.bin/top/display.c:1097
#6  0x804a3ca in u_process (line=1, 
    newline=0x80835e0 "  235 root       2    0   372K 1232K sleep     0:17  0.00%  0.00% sshd") at /home/itojun/NetBSD/src/usr.bin/top/display.c:721
#7  0x8048b8d in main (argc=3, argv=0xbfbfdae8)
    at /home/itojun/NetBSD/src/usr.bin/top/top.c:560
#8  0x80481c5 in ___start ()


Index: 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 01:49:07
@@ -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: 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 01:49:08
@@ -345,6 +345,7 @@
 		 */
 		if (limit != NULL && (*limit < i)) {
 			errno = E2BIG;
+			free(s);
 			return NULL;
 		}
   	
@@ -352,10 +353,11 @@
 		*area += i + 1;
 		if (limit != NULL) *limit -= i;
   	
-		return (s);
+		return (s);	/*XXX is it correct?*/
 	} else {
 		_DIAGASSERT(limit != NULL);
 		*limit = i;
+		free(s);
 		return NULL;
 	}
 }