Aren
/
NetBSD
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

PR/10266: t_getstr() leaks memory. This PR will stay in feedback 

until the problem gets addressed properly. The following fix
is a stopgap measure to stop the leaking :-(

I fixed the t_getstr() memory leak problem, but that instantly
revealed a problem in t_agetstr() which is an extremely broken
interface. It realloc's memory, potentially moving the area where
it returned pointers into in previous calls. This function needs
to be removed and or changed. I added a horrible work-around for
now, but I will revisit the problem shortly. In the meantime nobody
should be using the t_agetstr() API, and I'll be fixing the rest
of the programs and or the API when I figure out the best solution...
This is t_agetstr() is used by:

	games/hack/hack.termcap.c
	games/larn/io.c
	games/tetris/screen.c
	lib/libterm/termcap.c
	lib/libterm/termcap.h
	libexec/getty/main.c
	usr.bin/top/screen.c
	usr.bin/ul/ul.c
This commit is contained in:
christos 2001-10-31 21:52:17 +00:00
parent 66f972ab70
commit 961acd7fc2
1 changed files with 29 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
lib/libterm/termcap.c
View File

@ -1,4 +1,4 @@
/* $NetBSD: termcap.c,v 1.38 2001/01/29 01:22:31 christos Exp $ */
/* $NetBSD: termcap.c,v 1.39 2001/10/31 21:52:17 christos Exp $ */
/*
* Copyright (c) 1980, 1993
@ -38,7 +38,7 @@
#if 0
static char sccsid[] = "@(#)termcap.c 8.1 (Berkeley) 6/4/93";
#else
__RCSID("$NetBSD: termcap.c,v 1.38 2001/01/29 01:22:31 christos Exp $");
__RCSID("$NetBSD: termcap.c,v 1.39 2001/10/31 21:52:17 christos Exp $");
#endif
#endif /* not lint */
@ -101,9 +101,13 @@ t_setinfo(struct tinfo **bp, const char *entry)
cap_ptr = capability;
limit = 255;
(*bp)->up = t_getstr(*bp, "up", &cap_ptr, &limit);
if ((*bp)->up)
(*bp)->up = strdup((*bp)->up);
cap_ptr = capability;
limit = 255;
(*bp)->bc = t_getstr(*bp, "bc", &cap_ptr, &limit);
if ((*bp)->bc)
(*bp)->bc = strdup((*bp)->bc);
return 0;
}
@ -240,9 +244,13 @@ t_getent(bp, name)
cap_ptr = capability;
limit = 255;
(*bp)->up = t_getstr(*bp, "up", &cap_ptr, &limit);
if ((*bp)->up)
(*bp)->up = strdup((*bp)->up);
cap_ptr = capability;
limit = 255;
(*bp)->bc = t_getstr(*bp, "bc", &cap_ptr, &limit);
if ((*bp)->bc)
(*bp)->bc = strdup((*bp)->bc);
}
return (i + 1);
@ -354,8 +362,6 @@ tgetflag(id)
* placed in area, which is a ref parameter which is updated.
* limit is the number of characters allowed to be put into
* area, this is updated.
*
* returns dynamically allocated region, passed from cgetstr().
*/
char *
t_getstr(info, id, area, limit)
@ -390,7 +396,9 @@ t_getstr(info, id, area, limit)
return NULL;
}
strcpy(*area, s);
(void)strcpy(*area, s);
free(s);
s = *area;
*area += i + 1;
if (limit != NULL) *limit -= i;
@ -446,11 +454,17 @@ tgetstr(id, area)
* also returned to the caller. If the string is not found or a
* memory allocation fails then NULL is returned and bufptr and area
* are unchanged.
*
* XXX: This is horribly broken because realloc's can move memory and
* invalidate attributes we've already returned. Only possible way to
* fix it for now is to allocate a lot -- 8K, and leak if we need to
* realloc. This API needs to be destroyed!.
*/
#define BSIZE 8192
char *
t_agetstr(struct tinfo *info, const char *id, char **bufptr, char **area)
{
size_t new_size, offset;
size_t new_size;
char *new_buf;
_DIAGASSERT(info != NULL);
@ -461,26 +475,24 @@ t_agetstr(struct tinfo *info, const char *id, char **bufptr, char **area)
t_getstr(info, id, NULL, &new_size);
/* either the string is empty or the capability does not exist. */
if (new_size == 0)
if (new_size == 0 || new_size > BSIZE)
return NULL;
/* check if we have a buffer, if not malloc one and fill it in. */
if (*bufptr == NULL) {
if ((new_buf = (char *) malloc(new_size)) == NULL)
if ((new_buf = (char *) malloc(BSIZE)) == NULL)
return NULL;
*bufptr = new_buf;
*area = new_buf;
} else {
offset = *area - *bufptr;
if ((new_buf = realloc(*bufptr, offset + new_size)) == NULL)
return NULL;
*bufptr = new_buf;
*area = *bufptr + offset; /* we need to do this just in case
realloc shifted the buffer. */
} else if (*area - *bufptr >= BSIZE - new_size) {
char buf[BSIZE];
char *b = buf;
size_t len = BSIZE;
/* Leak! */
return strdup(t_getstr(info, id, &b, &len));
}
return t_getstr(info, id, area, NULL);
}
/*