* David Laight <david@l8s.co.uk> [2003/01/30 14:29]:
> > > 
> > > AT line 95 the code has:
> > > 
> > > 	if (strlen(c) >= l_value) {     /* old larger; copy over */
> > > 		while ((*c++ = *value++) != '\0');
> > > 		rwlock_unlock(&__environ_lock);
> > > 		return (0);
> > > 	} 
> > > 
> > > This isn't safe anymore....
> > 
> > Isn't it?  I guess I'm missing something then as I don't see it.
> > Probably not wise to be setenv()ing a variable previously putenv()ed
> > but it doesn't appear any more unsafe than it was...
> 
> Ah, but before the memory you we overwriting had always been allocated
> by the setenv code - it didn't belong to a user.

Well, wasn't this always the case?  You were not guaranteed that
the memory was allocated by the setenv code -- You might have
inherited a "static" environment, correct?.

> 
> > > I'm also not 100% about how the code behaves in the presence of
> > > environment string that don't contain an '=' (eg after putenv("fred");).
> > > 
> > > It needs to do something sensible.
> > 
> > It should probably return -1 like the current putenv() function.
> > I don't see anything in the standard that prevents this.  I'll
> > check what other implementations do.
> 
> That doesn't help, you cannot rely on the memory area not being overwritten
> by the application after putenv has been called.

I'm not sure I understand your point here.  Unless you mean what
is pointed out in the XPG/POSIX Application Usage section: 'A
potential error is to call putenv() with an automatic variable as
the argument, then return from the calling function while string
is still part of the environment.'

I do think that the current implementation (and it looks like most
BSD implementations) are might wrong to return -1.  I don't think
this fits with the XPG/POSIX specification.  A -1 return implies
that errno is set to something? what?  XPB/POSIX only specifies
ENOMEM.  I suppose it could follow the lead of setenv and set errno
to EINVAL.  The spec also says 'The string argument should point
to a string of the form "name=value"'.  I believe that the key word
is 'should'.


> 
> Something 'sensible' is all that is required...

Well, 'sensible' maybe...  It appears that System V accepts 
putenv("STRING"), and just places the string in the environment.
I'm not sure if this is 'sensible' or not especially since it
treats putenv("STRING") and putenv("STRING=") differently.
So your environ could look like:
	environ[0] = "STRING"
	environ[1] = "STRING="
IMHO this does not seem 'sensible'.

GNU libc takes a different approach.  It accepts putenv("STRING")
but then does an unsetenv("STRING").  Now depending upon what
you are expecting this may or may not be sensible.  Again this
may or may not be 'sensible'.

> FWIW in setenv, I'd do p = malloc(strlen(name) + strlen(value) + 2)
> without worrying about allocatin an extra byte if name ends or value
> starts with an '='.
> 
> To free the memory allocated by setenv, I'd suggest keeping another
> list of the memory blocks allocated by setenv.  Whenever something is
> freed, check the address matches.  The same index will work...
> That will bound the memory use even if parts of the application
> explicitly overwrite entries in the environ vector.
> 

It looks like Masaru OKI (Subject: unsetenv memory leak fix) has
attempted to do just this.  I don't think he's successfully gotten
it yet.  

> 	David

Brian
-- 
Brian Ginsbach                          Cray Inc.