I just fixed a memory leak in t_getstr() which in turn revealed a
fundamental flaw in t_agetstr(). If t_agetstr() needs to realloc()
memory, then all the pointers it has returned so far, need to be
adjusted for the possible memory block move. Although this is
documented in the man page, it is suboptimal. Not many programs
use that API, but the ones they do, do not pay attention to possible
block moves. So I suggest that we change libtermcap's t_agetent()
to automatically handle the memory allocation, and have t_freent()
free it.

This changes the footprint of t_agetent() from:

     char *t_agetstr(struct tinfo *info, char *id, char **buffer, char **area);
to:
     char *t_agetstr(struct tinfo *info, char *id);

I am planning to make this change without a major number bump, because
old code will still work the way it is. It will pass extra arguments
which will be ignored. (I will change the minor number).

Please let me know if you see any problems.

Thanks,

christos

Here's a preliminary version of the patch:

Index: termcap.3
===================================================================
RCS file: /cvsroot/basesrc/lib/libterm/termcap.3,v
retrieving revision 1.21
diff -u -u -r1.21 termcap.3
--- termcap.3	2001/01/05 23:05:08	1.21
+++ termcap.3	2001/11/01 16:05:44
@@ -74,7 +74,7 @@
 .Ft char *
 .Fn t_getstr "struct tinfo *info" "char *id" "char **area" "size_t *limit"
 .Ft char *
-.Fn t_agetstr "struct tinfo *info" "char *id" "char **buffer" "char **area"
+.Fn t_agetstr "struct tinfo *info" "char *id"
 .Ft int
 .Fn t_getterm "struct tinfo *info" "char **area" "size_t *limit"
 .Ft int
@@ -342,55 +342,11 @@
 .Fn t_agetstr
 performs the same function as 
 .Fn t_getstr
-except sufficient memory is first allocated to hold the desired string
-argument before adding it to the buffer.  To initialise the buffer for 
+except it handles memory allocation automatically. The memory that
 .Fn t_agetstr
-the first call must be done with the contents of
-.Fa buffer
-set to NULL.  Subsequent calls to 
-.Fn t_agetstr
-can pass
-.Fa buffer
-and
-.Fa area
-in unmodified and the requested string argument will be appended to
-the buffer after sufficient memory has been allocated.  Modifying either
-.Fa buffer
-or
-.Fa area
-between calls to 
-.Fn t_agetstr
-will certainly lead to the function misbehaving.  When the storage
-allocated by
-.Fn t_agetstr
-is no longer required it can be freed by passing the pointer contained
-in
-.Fa buffer
-to
-.Fn free .
-If an error occurs within
-.Fn t_agetstr
-a NULL will be returned and 
-.Fa buffer
-and
-.Fa area
-will remain unmodified.
-.Em NOTE
-.Fn t_agetstr
-uses
-.Fn realloc
-to reallocate the buffer memory so saving a copy of either 
-.Fa buffer
-or
-.Fa area
-for use after subsequent calls to 
-.Fn t_agetstr
-is likely to fail.  It is best to keep offsets from
-.Fa buffer
-to the desired string and calculate pointer addresses only after all
-the calls to
-.Fn t_agetstr
-have been done.
+allocates will be freed when
+.Fn t_freent
+is called.
 .Pp
 The function
 .Fn t_getterm
Index: termcap.c
===================================================================
RCS file: /cvsroot/basesrc/lib/libterm/termcap.c,v
retrieving revision 1.39
diff -u -u -r1.39 termcap.c
--- termcap.c	2001/10/31 21:52:17	1.39
+++ termcap.c	2001/11/01 16:05:44
@@ -108,6 +108,7 @@
 	(*bp)->bc = t_getstr(*bp, "bc", &cap_ptr, &limit);
 	if ((*bp)->bc)
 		(*bp)->bc = strdup((*bp)->bc);
+	(*bp)->tbuf = NULL;
 	
 	return 0;
 }
@@ -251,6 +252,7 @@
 		(*bp)->bc = t_getstr(*bp, "bc", &cap_ptr, &limit);
 		if ((*bp)->bc)
 			(*bp)->bc = strdup((*bp)->bc);
+		(*bp)->tbuf = NULL;
 	}
 		
 	return (i + 1);
@@ -460,39 +462,42 @@
  * 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
+#define BSIZE 512
 char *
-t_agetstr(struct tinfo *info, const char *id, char **bufptr, char **area)
+t_agetstr(struct tinfo *info, const char *id)
 {
 	size_t new_size;
-	char *new_buf;
+	struct tbuf *tb;
 	
 	_DIAGASSERT(info != NULL);
 	_DIAGASSERT(id != NULL);
-	_DIAGASSERT(bufptr != NULL);
-	_DIAGASSERT(area != NULL);
 
 	t_getstr(info, id, NULL, &new_size);
 
-	  /* either the string is empty or the capability does not exist. */
-	if (new_size == 0 || new_size > BSIZE)
+	/* either the string is empty or the capability does not exist. */
+	if (new_size == 0)
 		return NULL;
 
-	  /* check if we have a buffer, if not malloc one and fill it in. */
-	if (*bufptr == NULL) {
-		if ((new_buf = (char *) malloc(BSIZE)) == NULL)
+	if ((tb = info->tbuf) == NULL || (tb->eptr - tb->ptr) < new_size) {
+		if (new_size < BSIZE)
+			new_size = BSIZE;
+
+		if ((tb = malloc(sizeof(*info->tbuf))) == NULL)
+			return NULL;
+
+		if ((tb->data = tb->ptr = tb->eptr = malloc(new_size)) == NULL)
 			return NULL;
-		*bufptr = new_buf;
-		*area = new_buf;
-	} 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));
+
+		tb->eptr += new_size;
+
+		if (info->tbuf != NULL)
+			tb->next = info->tbuf;
+		else
+			tb->next = NULL;
+
+		info->tbuf = tb;
 	}
-	return t_getstr(info, id, area, NULL);
-	
+	return t_getstr(info, id, &tb->ptr, NULL);
 }
  
 /*
@@ -503,12 +508,18 @@
 t_freent(info)
 	struct tinfo *info;
 {
+	struct tbuf *tb;
 	_DIAGASSERT(info != NULL);
 	free(info->info);
 	if (info->up != NULL)
 		free(info->up);
 	if (info->bc != NULL)
 		free(info->bc);
+	for (tb = info->tbuf; tb;) {
+		tb = info->tbuf->next;
+		free(info->tbuf->data);
+		free(info->tbuf);
+	}
 	free(info);
 }
 
Index: termcap.h
===================================================================
RCS file: /cvsroot/basesrc/lib/libterm/termcap.h,v
retrieving revision 1.12
diff -u -u -r1.12 termcap.h
--- termcap.h	2000/05/28 09:58:15	1.12
+++ termcap.h	2001/11/01 16:05:45
@@ -60,7 +60,7 @@
 int   t_getnum  __P((struct tinfo *, const char *));
 int   t_getflag __P((struct tinfo *, const char *));
 char *t_getstr  __P((struct tinfo *, const char *, char **, size_t *));
-char *t_agetstr(struct tinfo *, const char *, char **, char **);
+char *t_agetstr(struct tinfo *, const char *);
 int   t_getterm(struct tinfo *, char **, size_t *);
 int   t_goto    __P((struct tinfo *, const char *, int, int, char *, size_t));
 int   t_puts    __P((struct tinfo *, const char *, int,
Index: termcap_private.h
===================================================================
RCS file: /cvsroot/basesrc/lib/libterm/termcap_private.h,v
retrieving revision 1.3
diff -u -u -r1.3 termcap_private.h
--- termcap_private.h	2001/06/13 10:46:00	1.3
+++ termcap_private.h	2001/11/01 16:05:45
@@ -35,9 +35,12 @@
 struct tinfo
 {
 	char *info;
-	char *up; /* for use by tgoto */
-	char *bc; /* for use by tgoto */
+	char *up; 			/* for use by tgoto */
+	char *bc; 			/* for use by tgoto */
+	struct tbuf {			/* for use by t_agetstr() */
+		struct tbuf *next;	/* pointer to next area */
+		char *data;		/* pointer to beginning of buffer */
+		char *ptr;		/* current data pointer */
+		char *eptr;		/* pointer to the end of buffer */
+	} *tbuf;
 };
-
-
-