[MPlayer-dev-eng] [PATCH] move setenv implementation to osdep/
Rich Felker
dalias at aerifal.cx
Thu Oct 27 18:11:44 CEST 2005
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
More information about the MPlayer-dev-eng
mailing list