[MPlayer-dev-eng] [PATCH] move setenv implementation to osdep/

[Rich Felker]

On Thu, Oct 27, 2005 at 11:30:03AM +0200, Alexander Strasser wrote:
> Hi,
> 
>   maybe I am missing something fundamentally but
> it did not work at all for me.
>   Comments follow, i also attached an updated
> patch. Not too well tested so comments and testers
> appreciated.
> 
> Diego Biurrun wrote:
> > On Thu, Oct 27, 2005 at 12:29:15AM +0200, Diego Biurrun wrote:
> > > Attached patch should fix bug #342 and generally make sense by removing
> > > code duplication.
> > > 
> > > Any glaring mistakes?  OK to commit?
> >   ^^^^^^^^^^^^^^^^^^^^^
> [...]
> > Index: configure
> > ===================================================================
> > +/* Define this if your system has setenv */
> > +$_def_setenv
> > +
> 
>   I think the systems missing setenv don't have a
> prototype in stdlib.h for it, so we need to provide
> one else where.

Why? Nothing says you need a prototype. In fact setenv has the default
return type (int) so it's especially useless.

> [...]
> > --- /dev/null	2005-10-26 22:03:01.101616344 +0200
> > +++ osdep/setenv.c	2005-10-27 00:21:49.000000000 +0200
> > @@ -0,0 +1,20 @@
> > +/* setenv implementation for systems lacking setenv in stdlib.h. */
> > +
> > +#ifndef HAVE_SETENV
> > +
> > +#include <stdlib.h>
> > +#include <string.h>
> > +
> > +static void setenv(const char *name, const char *val, int _xx)
> 
>   Static is not the appropriate keyword here. I also don't like
> to have a different signature for setenv, at least on my system
> it returns int. If this is not normally implemented that way
> i take back this one.

Yes, void is wrong.

> > +{
> > +    int len  = strlen(name) + strlen(val) + 2;
> > +    char *env = malloc(len);
> > +
> > +    if (env != NULL) {
> > +			  strcpy(env, name);
> > +			  strcat(env, "=");
> > +			  strcat(env, val);
> > +			  putenv(env);
> > +    }
> > +}
> > +#endif
> 
>   Also completely ignoring the last parameter seems to be
> a bad solution. I added an assert to assure it is always
> nonzero so at least with debug builds one can notice there
> is something wrong. 

Agree.

Rich