[ BTW, for some reason i couldn't get your patch to actually apply... ]

"Simon J. Gerraty" <sjg@quick.com.au> writes:
>                 for (p = environ, cnt = 0; *p; ++p, ++cnt);
>                 if (alloced) {                  /* just increase size */
> -                       environ = realloc(environ,
> -                           (size_t)(sizeof(char *) * (cnt + 2)));
> -                       if (!environ) {
> +                       p = realloc(environ,
> +                                   (size_t)(sizeof(char *) * (cnt + 2)));
> +                       if (!p) {

I think the "p != NULL" form is better style for this test.

>                                 rwlock_unlock(&__environ_lock);
>                                 return (-1);
>                         }
> +                       environ = p;
>                 }
>                 else {                          /* get new space */
>                         alloced = 1;            /* copy old entries into it */

I looked at the 'else' (! alloced) case as well, and i think it has a
bug: if the malloc fails, alloced will be inappropriately set to 1.

Also, really, the two cases can be collapsed since realloc(NULL, ...)
is allowed by c89 (and we're now allowing c89 8-).

What do you think about the diff below (based on yours 8-)?  I've not
tried even compiling it...  8-)



chris
===================================================================
Index: setenv.c
===================================================================
RCS file: /cvsroot/basesrc/lib/libc/stdlib/setenv.c,v
retrieving revision 1.19
diff -c -r1.19 setenv.c
*** setenv.c	2000/12/20 18:38:30	1.19
--- setenv.c	2001/03/12 19:48:19
***************
*** 101,124 ****
  		char **p;
  
  		for (p = environ, cnt = 0; *p; ++p, ++cnt);
! 		if (alloced) {			/* just increase size */
! 			environ = realloc(environ,
! 			    (size_t)(sizeof(char *) * (cnt + 2)));
! 			if (!environ) {
! 				rwlock_unlock(&__environ_lock);
! 				return (-1);
! 			}
  		}
! 		else {				/* get new space */
! 			alloced = 1;		/* copy old entries into it */
! 			p = malloc((size_t)(sizeof(char *) * (cnt + 2)));
! 			if (!p) {
! 				rwlock_unlock(&__environ_lock);
! 				return (-1);
! 			}
  			memcpy(p, environ, cnt * sizeof(char *));
! 			environ = p;
  		}
  		environ[cnt + 1] = NULL;
  		offset = cnt;
  	}
--- 101,122 ----
  		char **p;
  
  		for (p = environ, cnt = 0; *p; ++p, ++cnt);
! 
! 		/* Allocate the space for the larger list. */
! 		p = realloc(alloced ? environ : NULL,
! 		    (size_t)(sizeof(char *) * (cnt + 2)));
! 		if (p == NULL) {
! 			rwlock_unlock(&__environ_lock);
! 			return (-1);
  		}
! 
! 		/* If new allocation, copy entries from environ. */
! 		if (!alloced) {
  			memcpy(p, environ, cnt * sizeof(char *));
! 			alloced = 1;
  		}
+ 
+ 		environ = p;
  		environ[cnt + 1] = NULL;
  		offset = cnt;
  	}