On Fri, 27 Jun 2003 18:42:27 +0100
Ben Harris <bjh21@netbsd.org> wrote:

> This could be improved by remembering the address of the block that was
> allocated by setenv, and only trying to free anything if the address hasn't
> changed by the time unsetenv is called.

Thank you for comments, and another bug was found by my own review.
I made new patch.  It depends on the offset.

-- 
Masaru OKI <oki@netbsd.org>

Index: __unsetenv13.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/__unsetenv13.c,v
retrieving revision 1.2
diff -u -r1.2 __unsetenv13.c
--- __unsetenv13.c	2003/04/13 17:39:13	1.2
+++ __unsetenv13.c	2003/06/28 07:07:07
@@ -68,6 +68,8 @@
 #endif
 
 extern char **environ;
+extern int __environ_alloced;
+extern char *__malloced_env;
 
 /*
  * unsetenv(name) --
@@ -81,7 +83,7 @@
 unsetenv(name)
 	const char *name;
 {
-	char **p;
+	char **p, **mp;
 	int offset;
 
 	_DIAGASSERT(name != NULL);
@@ -94,10 +96,16 @@
 #endif
 
 	rwlock_wrlock(&__environ_lock);
-	while (__findenv(name, &offset))	/* if set multiple times */
-		for (p = &environ[offset];; ++p)
+	while (__findenv(name, &offset)) {	/* if set multiple times */
+		if (__environ_alloced == 1 && __malloced_env[offset] != 0)
+			free(__malloced_env[offset]);
+		for (p = &environ[offset], mp = &__malloced_env[offset];;
+		    ++p, ++mp) {
 			if (!(*p = *(p + 1)))
 				break;
+			*mp = *(mp + 1);
+		}
+	}
 	rwlock_unlock(&__environ_lock);
 
 #ifndef __LIBC12_SOURCE__
Index: setenv.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/setenv.c,v
retrieving revision 1.22
diff -u -r1.22 setenv.c
--- setenv.c	2003/04/07 13:41:14	1.22
+++ setenv.c	2003/06/28 07:07:08
@@ -61,6 +61,9 @@
 
 extern char **environ;
 
+int __environ_alloced;			/* if allocated space before */
+char *__malloced_env;
+
 /*
  * setenv --
  *	Set the value of the environmental variable "name" to be
@@ -72,7 +75,6 @@
 	const char *value;
 	int rewrite;
 {
-	static int alloced;			/* if allocated space before */
 	char *c;
 	const char *cc;
 	size_t l_value;
@@ -101,21 +103,34 @@
 		char **p;
 
 		for (p = environ, cnt = 0; *p; ++p, ++cnt);
-		if (alloced) {			/* just increase size */
+		if (__environ_alloced) {	/* just increase size */
 			environ = realloc(environ,
 			    (size_t)(sizeof(char *) * (cnt + 2)));
 			if (!environ) {
 				rwlock_unlock(&__environ_lock);
 				return (-1);
 			}
+			__malloced_env = realloc(__malloced_env,
+			    (size_t)(sizeof(char *) * (cnt + 2)));
+			if (!__malloced_env) {
+				rwlock_unlock(&__environ_lock);
+				return (-1);
+			}
+			__malloced_env[cnt] = 0;
 		}
 		else {				/* get new space */
-			alloced = 1;		/* copy old entries into it */
+			__environ_alloced = 1;	/* copy old entries into it */
 			p = malloc((size_t)(sizeof(char *) * (cnt + 2)));
 			if (!p) {
 				rwlock_unlock(&__environ_lock);
 				return (-1);
 			}
+			__malloced_env = calloc(cnt + 2, sizeof(char *));
+			if (!__malloced_env) {
+				free(p);
+				rwlock_unlock(&__environ_lock);
+				return (-1);
+			}
 			memcpy(p, environ, cnt * sizeof(char *));
 			environ = p;
 		}
@@ -124,11 +139,14 @@
 	}
 	for (cc = name; *cc && *cc != '='; ++cc)/* no `=' in name */
 		continue;
+	if (__malloced_env[offset] != 0)
+		free(environ[offset]);
 	if (!(environ[offset] =			/* name + `=' + value */
 	    malloc((size_t)((int)(cc - name) + l_value + 2)))) {
 		rwlock_unlock(&__environ_lock);
 		return (-1);
 	}
+	__malloced_env[offset] = environ[offset];
 	for (c = environ[offset]; (*c = *name++) && *c != '='; ++c);
 	for (*c++ = '='; (*c++ = *value++) != '\0'; );
 	rwlock_unlock(&__environ_lock);