HEADS DOWN

[Andrey Chernov]

On Fri, May 04, 2007 at 01:21:50PM -0500, Sean C. Farley wrote:
>  I believe I check all that you did in your changes.  Mine looks a little
>  different since some checks were combined for speed (i.e.,
>  __strleneq()).

Looking at 
http://www.farley.org/freebsd/tmp/setenv-8/POSIX/sysenv.c

__strleneq() - "Inline strlen() for performance." - no performance gain 
such way but lost, because strlen() already auto-inlined with gcc -O2 in 
much better assembler form:
 repnz
 scasb
So there is no point to call __strleneq(..., false); too instead of just 
strlen() (which will be auto-inlined by gcc) directly.

Currently __clean_env is unused. Why don't #if 0 it?

I think "Check for malformed name" should be before "Initialize 
environment" - with malformed arg complex environment mallocing/copying
initialization ever not needed.

In __build_env() when strdup() returns NULL, you better free all previous 
strdups in that loop and envVars too before returning (-1)

Why you use strstr(envVars[envNdx].name, "="); instead of
strchr(envVars[envNdx].name, '='); ? In theory strchr() can be gcc-inlined 
or assembler-implemented with more probability than strstr() (currently 
both are not inlined at i386).

__rebuild_environ() needs to use realloc() instead of reallocf() 
because realloc() not touch original pointer/content when fails. I.e. 
_whole_ "environ" will not be lost in case can't be enlarged. Something 
like that:

		TempEnvironSize = newEnvSize * 2;
		TempEnviron = realloc(environ, sizeof (*environ) * 
				(TempEnvironSize +  1));
		if (TempEnviron == NULL)
			return (-1);
		environ = TempEnviron;
		environSize = TempEnvironSize;

>  The only other question I have is about leading whitespace in the name
>  passed to *env():
>  1. Should it be removed up to the first non-whitespace character?
>  2. Treated as part of the variable name.  Is this allowed by the
>     specification?
>  3. Return errno = EINVAL.  getenv() would just return NULL.

POSIX says about names:
1) "names shall not contain the character '='" and "points to an empty 
string".
2) "Other characters may be permitted by an implementation; applications 
shall tolerate the presence of such names."
So I see no point to treat ' ' some special way.

-- 
http://ache.pp.ru/