httpd: memory hole in function addEnv() and more

Rob Landley rob at landley.net
Sun Sep 4 04:12:18 UTC 2005


On Saturday 03 September 2005 19:00, Dirk Clemens wrote:
> Them memory alloced by asprintf() is never free'd.
> Solution: insert free(s) behind putenv(s).

And since this could be used repeatedly from a long-running script, it's not 
even a FEATURE_CLEAN_UP free, either.

Too bad there's no obvious way to apply alloca() here.  Hmmm...

> 2.) interface of addEnv()
>
> addEnv() takes 2 parameters for bulding the variable name.

A very silly wrapper, yes.

> If we use this we need also some modification of addEnvPort()
>
> static void addEnvPort(const char *name)
> {
>       char buf[16];
>
>       sprintf(buf, "%u", config->port);
>       addEnv(name, buf);
> }

Okay, this function just seems unnecessary.  Just inline the following at the 
two actual call sites:

if(ENABLE_FEATURE_HTTPD_SET_REMOTE_PORT_TO_ENV)
{
 char buf[32];
 sprintf("REMOTE=%u",config->port);
 putenv(buf);
}

And

if(ENABLE_FEATURE_HTTPD_USAGE_FROM_INETD_ONLY)
{
 char buf[32];
 sprintf("SERVER=%u",config->port);
 putenv(buf);
}

Then zap  addEnvPort entirely.

> 3.) usage of addEnv()
> In some source positions I found callings like
>       addEnv("SERVER",         "PROTOCOL", "HTTP/1.0");
> with 3 string constants.
> I think that we can save memory and cpu time if we change such lines into
>     putenv("SERVER_PROTOCOL=HTTP/1.0");

I heartily support this cleanup. :)

> In my opinion it does not make sense:
>       addEnv("PATH",           "",         getenv("PATH"));

Nope, it doesn't...

Go for it.  I look forward to the patch.

Rob



More information about the busybox mailing list