* putenv()/environ bug @ 2007-07-25 15:09 Sean C. Farley 2007-07-25 20:53 ` Peter Stephenson 0 siblings, 1 reply; 11+ messages in thread From: Sean C. Farley @ 2007-07-25 15:09 UTC (permalink / raw) To: zsh-workers As noticed here[1] following a change[2] in FreeBSD's *env() functions, zsh is mixing *env() (putenv() in this case) functions with direct access to the environ variable's contents against the IEEE Std 1003.1 specification. For the FreeBSD port, there is an easy solution in the thread to tell configure that putenv() does not exist. I just wanted to let the zsh development team know about this issue. Regarding FreeBSD, if you replace the environ pointer, a call to setenv(), putenv() or unsetenv() will detect this and update itself internally. getenv() will use environ until one of the other *env() functions is called. If you manipulate environ values after a call to setenv(), putenv() or unsetenv() is made, then the change will go unnoticed. BTW, is there a particular reason the standard *env() functions cannot be used for all operations to environ if found? Sean 1. http://lists.freebsd.org/pipermail/freebsd-current/2007-July/075541.html 2. http://lists.freebsd.org/pipermail/freebsd-ports/2007-May/041577.html -- scf@FreeBSD.org ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: putenv()/environ bug 2007-07-25 15:09 putenv()/environ bug Sean C. Farley @ 2007-07-25 20:53 ` Peter Stephenson 2007-07-26 0:14 ` Sean C. Farley 0 siblings, 1 reply; 11+ messages in thread From: Peter Stephenson @ 2007-07-25 20:53 UTC (permalink / raw) To: zsh-workers; +Cc: Sean C. Farley On Wed, 25 Jul 2007 10:09:22 -0500 (CDT) "Sean C. Farley" <scf@FreeBSD.org> wrote: > As noticed here[1] following a change[2] in FreeBSD's *env() functions, > zsh is mixing *env() (putenv() in this case) functions with direct > access to the environ variable's contents against the IEEE Std 1003.1 > specification. > > BTW, is there a particular reason the standard *env() functions cannot > be used for all operations to environ if found? There's a long history of fiddling with these for problems on various systems, so I'm a little unwilling to change it without some guidance. For example, /* * Under Cygwin we must use putenv() to maintain consistency. * Unfortunately, current version (1.1.2) copies argument and may * silently reuse existing environment string. This tries to * check for both cases */ This is a little confusing since the code in question (addenv in params.c) doesn't actually use putenv(). Given we manipulate environ quite a lot anyway, is there any harm in using only the zsh versions of zgetenv() and zputenv()? There's a getenv() instead of a zgetenv() in init.c: I think that was just a typo by me. Index: Src/init.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/init.c,v retrieving revision 1.76 diff -u -r1.76 init.c --- Src/init.c 6 Jul 2007 21:52:39 -0000 1.76 +++ Src/init.c 25 Jul 2007 20:53:06 -0000 @@ -832,7 +832,7 @@ if (emulation == EMULATE_ZSH) ptr = home; else - ptr = getenv("HOME"); + ptr = zgetenv("HOME"); if (ptr && ispwd(ptr)) pwd = ztrdup(ptr); else if ((ptr = zgetenv("PWD")) && (strlen(ptr) < PATH_MAX) && -- Peter Stephenson <p.w.stephenson@ntlworld.com> Web page now at http://homepage.ntlworld.com/p.w.stephenson/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: putenv()/environ bug 2007-07-25 20:53 ` Peter Stephenson @ 2007-07-26 0:14 ` Sean C. Farley 2007-07-28 18:46 ` Andrey Borzenkov 0 siblings, 1 reply; 11+ messages in thread From: Sean C. Farley @ 2007-07-26 0:14 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers On Wed, 25 Jul 2007, Peter Stephenson wrote: > On Wed, 25 Jul 2007 10:09:22 -0500 (CDT) > "Sean C. Farley" <scf@FreeBSD.org> wrote: >> As noticed here following a change in FreeBSD's *env() functions, zsh >> is mixing *env() (putenv() in this case) functions with direct access >> to the environ variable's contents against the IEEE Std 1003.1 >> specification. >> >> BTW, is there a particular reason the standard *env() functions >> cannot be used for all operations to environ if found? > > There's a long history of fiddling with these for problems on various > systems, so I'm a little unwilling to change it without some guidance. > > For example, > > /* > * Under Cygwin we must use putenv() to maintain consistency. > * Unfortunately, current version (1.1.2) copies argument and may > * silently reuse existing environment string. This tries to > * check for both cases > */ I understand. > This is a little confusing since the code in question (addenv in > params.c) doesn't actually use putenv(). Legacy comments are only meant to throw developers off the track. :) > Given we manipulate environ quite a lot anyway, is there any harm in > using only the zsh versions of zgetenv() and zputenv()? There's a > getenv() instead of a zgetenv() in init.c: I think that was just a > typo by me. *code snipped* Unfortunately, this does not fix the problem. unsets are only affecting the zsh environment but not environ. For example, here is a reduced set of calls to duplicate it: export FOO=BAR exec zsh unset FOO env | grep FOO echo $FOO The call to env (/bin/env) will show "FOO=BAR" while the echo will not. It is difficult to decide what to do. Here are some thoughts in my head: 1. Telling zsh that putenv() does not exist forces it to manipulate a new copy of environ (zputenv() and createparamtable()). This almost (see #2) prevents it from mixing *env() functions with direct changes to environ. 2. To truly prevent mixing, zsh may need to be told getenv() does not exist. This way it is not directly (see #3) calling any *env() functions. I have not seen a need (any bug reports) for this. 3. There may be other libc functions that call *env() functions. For example, setlocale() calls getenv(). If all setlocale() calls are done prior to any manipulation of environ, then this should be safe. 4. The FreeBSD *env() code can detect if environ != prevEnviron but not if environ[x] has changed. 5. OpenSolaris does some similar stuff with its *env() functions[1]. Somebody may want to check if it is also affected. 6. The FreeBSD port[2] should have a fix relatively soon to avoid this by telling configure not to look for putenv(). Sean 1. http://cvs.opensolaris.org/source/xref/clearview/usr/src/lib/libc/port/gen/getenv.c 2. http://www.freebsd.org/cgi/cvsweb.cgi/ports/shells/zsh/ -- scf@FreeBSD.org ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: putenv()/environ bug 2007-07-26 0:14 ` Sean C. Farley @ 2007-07-28 18:46 ` Andrey Borzenkov 2007-07-29 19:08 ` Sean C. Farley 0 siblings, 1 reply; 11+ messages in thread From: Andrey Borzenkov @ 2007-07-28 18:46 UTC (permalink / raw) To: zsh-workers; +Cc: Sean C. Farley [-- Attachment #1: Type: text/plain, Size: 2261 bytes --] On Thursday 26 July 2007, Sean C. Farley wrote: > On Wed, 25 Jul 2007, Peter Stephenson wrote: > > On Wed, 25 Jul 2007 10:09:22 -0500 (CDT) > > > > "Sean C. Farley" <scf@FreeBSD.org> wrote: > >> As noticed here following a change in FreeBSD's *env() functions, zsh > >> is mixing *env() (putenv() in this case) functions with direct access > >> to the environ variable's contents against the IEEE Std 1003.1 > >> specification. > >> > >> BTW, is there a particular reason the standard *env() functions > >> cannot be used for all operations to environ if found? > > The code in question is quite old and I believe it predates (un-)setenv. And you simply did not have any way to *unset* environment string in some systems; I think (un-)setenv is BSD extension. > > There's a long history of fiddling with these for problems on various > > systems, so I'm a little unwilling to change it without some guidance. > > > > For example, > > > > /* > > * Under Cygwin we must use putenv() to maintain consistency. > > * Unfortunately, current version (1.1.2) copies argument and may > > * silently reuse existing environment string. This tries to > > * check for both cases > > */ > > I understand. > > > This is a little confusing since the code in question (addenv in > > params.c) doesn't actually use putenv(). > > Legacy comments are only meant to throw developers off the track. :) > Huh? addenv() is using zputenv() that is using putenv() where avaialable. Now it may be legacy in the sense Cygwin implemenation has changed; but unfortunately I use Cygwin no more nor have environment to check it. > > Given we manipulate environ quite a lot anyway, is there any harm in > > using only the zsh versions of zgetenv() and zputenv()? There's a > > getenv() instead of a zgetenv() in init.c: I think that was just a > > typo by me. > > *code snipped* > > Unfortunately, this does not fix the problem. unsets are only affecting > the zsh environment but not environ. What about - check if (un-)setenv is available and use them. On legacy systems use existing implementation. This probably will mean we will be using native utilities everywhere on modern systems. -andrey [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: putenv()/environ bug 2007-07-28 18:46 ` Andrey Borzenkov @ 2007-07-29 19:08 ` Sean C. Farley 2007-07-30 20:39 ` Peter Stephenson 0 siblings, 1 reply; 11+ messages in thread From: Sean C. Farley @ 2007-07-29 19:08 UTC (permalink / raw) To: Andrey Borzenkov; +Cc: zsh-workers On Sat, 28 Jul 2007, Andrey Borzenkov wrote: > On Thursday 26 July 2007, Sean C. Farley wrote: >> On Wed, 25 Jul 2007, Peter Stephenson wrote: >>> On Wed, 25 Jul 2007 10:09:22 -0500 (CDT) >>> >>> "Sean C. Farley" <scf@FreeBSD.org> wrote: >>>> As noticed here following a change in FreeBSD's *env() functions, >>>> zsh is mixing *env() (putenv() in this case) functions with direct >>>> access to the environ variable's contents against the IEEE Std >>>> 1003.1 specification. >>>> >>>> BTW, is there a particular reason the standard *env() functions >>>> cannot be used for all operations to environ if found? >>> > > The code in question is quite old and I believe it predates > (un-)setenv. And you simply did not have any way to *unset* > environment string in some systems; I think (un-)setenv is BSD > extension. OK. That would be old. I cannot think of any relatively current system that does not have unsetenv(). >>> There's a long history of fiddling with these for problems on >>> various systems, so I'm a little unwilling to change it without some >>> guidance. >>> >>> For example, >>> >>> /* >>> * Under Cygwin we must use putenv() to maintain consistency. >>> * Unfortunately, current version (1.1.2) copies argument and may >>> * silently reuse existing environment string. This tries to >>> * check for both cases >>> */ >> >> I understand. >> >>> This is a little confusing since the code in question (addenv in >>> params.c) doesn't actually use putenv(). >> >> Legacy comments are only meant to throw developers off the track. :) > > Huh? addenv() is using zputenv() that is using putenv() where > avaialable. Now it may be legacy in the sense Cygwin implemenation has > changed; but unfortunately I use Cygwin no more nor have environment > to check it. Well, the comment comes after the use of zputenv(). The block of code under the comment does not use *putenv(). >>> Given we manipulate environ quite a lot anyway, is there any harm in >>> using only the zsh versions of zgetenv() and zputenv()? There's a >>> getenv() instead of a zgetenv() in init.c: I think that was just a >>> typo by me. >> >> *code snipped* >> >> Unfortunately, this does not fix the problem. unsets are only >> affecting the zsh environment but not environ. > > What about - check if (un-)setenv is available and use them. On legacy > systems use existing implementation. This probably will mean we will > be using native utilities everywhere on modern systems. That would work for me. If anyone would like me to test any patches, I will. Sean -- scf@FreeBSD.org ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: putenv()/environ bug 2007-07-29 19:08 ` Sean C. Farley @ 2007-07-30 20:39 ` Peter Stephenson 2007-07-30 20:52 ` Peter Stephenson 2007-07-30 22:39 ` Sean C. Farley 0 siblings, 2 replies; 11+ messages in thread From: Peter Stephenson @ 2007-07-30 20:39 UTC (permalink / raw) To: zsh-workers On Sun, 29 Jul 2007 14:08:42 -0500 (CDT) "Sean C. Farley" <scf@FreeBSD.org> wrote: > On Sat, 28 Jul 2007, Andrey Borzenkov wrote: > > What about - check if (un-)setenv is available and use them. On legacy > > systems use existing implementation. This probably will mean we will > > be using native utilities everywhere on modern systems. > > That would work for me. If anyone would like me to test any patches, I > will. This sounds the best, and it's working on Fedora 7. I've tested if both setenv() and unsetenv() are available and if they are we don't do any management of the environment ourselves, so the hack in addenv() doesn't occur... I hope that works, otherwise the implementation of the library is fairly well broken, the whole point of setenv/unsetenv being to be able to do this in a standard fashion. The only references to environ we make with this code are to read it at the start to initialise internval variables and to pass it to execs. This seems to be sanctioned by POSIX. As a TODO indicates, this means we don't actually need to store an "env" string in the parameter structure, just a flag saying we've stuck the value in the environment. That can change when this stuff is clearly working. Index: configure.ac =================================================================== RCS file: /cvsroot/zsh/zsh/configure.ac,v retrieving revision 1.66 diff -u -r1.66 configure.ac --- configure.ac 25 Jul 2007 14:54:57 -0000 1.66 +++ configure.ac 30 Jul 2007 20:36:27 -0000 @@ -1126,7 +1126,7 @@ setlocale \ uname \ signgam \ - putenv getenv \ + putenv getenv setenv unsetenv xw\ brk sbrk \ pathconf sysconf \ tgetent tigetflag tigetnum tigetstr setupterm \ Index: Src/exec.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/exec.c,v retrieving revision 1.119 diff -u -r1.119 exec.c --- Src/exec.c 13 Jul 2007 10:09:07 -0000 1.119 +++ Src/exec.c 30 Jul 2007 20:36:29 -0000 @@ -524,7 +524,16 @@ * that as argv[0] for this external command */ if (unset(RESTRICTED) && (z = zgetenv("ARGV0"))) { setdata(firstnode(args), (void *) ztrdup(z)); + /* + * Note we don't do anything with the parameter structure + * for ARGV0: that's OK since we're about to exec or exit + * on failure. + */ +#ifdef HAVE_UNSETENV + unsetenv("ARGV0"); +#else delenvvalue(z - 6); +#endif } else if (flags & BINF_DASH) { /* Else if the pre-command `-' was given, we add `-' * * to the front of argv[0] for this command. */ Index: Src/params.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/params.c,v retrieving revision 1.133 diff -u -r1.133 params.c --- Src/params.c 25 Jul 2007 09:26:51 -0000 1.133 +++ Src/params.c 30 Jul 2007 20:36:34 -0000 @@ -610,7 +610,7 @@ createparamtable(void) { Param ip, pm; -#ifndef HAVE_PUTENV +#if !defined(HAVE_PUTENV) && !defined(HAVE_SETENV) char **new_environ; int envsize; #endif @@ -665,7 +665,7 @@ setsparam("LOGNAME", ztrdup((str = getlogin()) && *str ? str : cached_username)); -#ifndef HAVE_PUTENV +#if !defined(HAVE_PUTENV) && !defined(HAVE_SETENV) /* Copy the environment variables we are inheriting to dynamic * * memory, so we can do mallocs and frees on it. */ envsize = sizeof(char *)*(1 + arrlen(environ)); @@ -3855,6 +3855,30 @@ int zputenv(char *str) { +#ifdef HAVE_SETENV + /* + * If we are using unsetenv() to remove values from the + * environment, which is the safe thing to do, we + * need to use setenv() to put them there in the first place. + * Unfortunately this is a slightly different interface + * from what zputenv() assumes. + */ + char *ptr; + int ret; + + for (ptr = str; *ptr && *ptr != '='; ptr++) + ; + if (*ptr) { + *ptr = '\0'; + ret = setenv(str, ptr+1, 1); + *ptr = '='; + } else { + /* safety first */ + DPUTS(1, "bad environment string"); + ret = setenv(str, ptr, 1); + } + return ret; +#else #ifdef HAVE_PUTENV return putenv(str); #else @@ -3878,9 +3902,12 @@ } return 0; #endif +#endif } /**/ +#ifndef HAVE_UNSETENV +/**/ static int findenv(char *name, int *pos) { @@ -3899,6 +3926,8 @@ return 0; } +/**/ +#endif /* Given *name = "foo", it searches the environment for string * * "foo=bar", and returns a pointer to the beginning of "bar" */ @@ -3939,14 +3968,20 @@ void addenv(Param pm, char *value) { - char *oldenv = 0, *newenv = 0, *env = 0; + char *newenv = 0; +#ifndef HAVE_UNSETENV + char *oldenv = 0, *env = 0; int pos; +#endif - /* First check if there is already an environment * - * variable matching string `name'. If not, and * - * we are not requested to add new, return */ +#ifndef HAVE_UNSETENV + /* + * First check if there is already an environment + * variable matching string `name'. + */ if (findenv(pm->node.nam, &pos)) oldenv = environ[pos]; +#endif newenv = mkenvstr(pm->node.nam, value, pm->node.flags); if (zputenv(newenv)) { @@ -3954,6 +3989,19 @@ pm->env = NULL; return; } +#ifdef HAVE_UNSETENV + /* + * If we are using setenv/unsetenv to manage the environment, + * we simply store the string we created in pm->env since + * memory management of the environment is handled entirely + * by the system. + * + * TODO: is this good enough to fix problem cases from + * the other branch? If so, we don't actually need to + * store pm->env at all, just a flag that the value was set. + */ + pm->env = newenv; +#else /* * Under Cygwin we must use putenv() to maintain consistency. * Unfortunately, current version (1.1.2) copies argument and may @@ -3973,6 +4021,7 @@ DPUTS(1, "addenv should never reach the end"); pm->env = NULL; +#endif } @@ -4003,6 +4052,7 @@ * string. */ +#ifndef HAVE_UNSETENV /**/ void delenvvalue(char *x) @@ -4018,6 +4068,8 @@ } zsfree(x); } +#endif + /* Delete a pointer from the list of pointers to environment * * variables by shifting all the other pointers up one slot. */ @@ -4026,7 +4078,12 @@ void delenv(Param pm) { +#ifdef HAVE_UNSETENV + unsetenv(pm->node.nam); + zsfree(pm->env); +#else delenvvalue(pm->env); +#endif pm->env = NULL; /* * Note we don't remove PM_EXPORT from the flags. This Index: Src/system.h =================================================================== RCS file: /cvsroot/zsh/zsh/Src/system.h,v retrieving revision 1.44 diff -u -r1.44 system.h --- Src/system.h 23 Apr 2007 15:15:13 -0000 1.44 +++ Src/system.h 30 Jul 2007 20:36:34 -0000 @@ -693,6 +693,15 @@ extern char **environ; +/* + * We always need setenv and unsetenv in pairs, because + * we don't know how to do memory management on the values set. + */ +#ifndef HAVE_UNSETENV +#undef HAVE_SETENV +#endif + + /* These variables are sometimes defined in, * * and needed by, the termcap library. */ #if MUST_DEFINE_OSPEED -- Peter Stephenson <p.w.stephenson@ntlworld.com> Web page now at http://homepage.ntlworld.com/p.w.stephenson/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: putenv()/environ bug 2007-07-30 20:39 ` Peter Stephenson @ 2007-07-30 20:52 ` Peter Stephenson 2007-07-30 22:44 ` Sean C. Farley 2007-07-30 22:39 ` Sean C. Farley 1 sibling, 1 reply; 11+ messages in thread From: Peter Stephenson @ 2007-07-30 20:52 UTC (permalink / raw) To: zsh-workers Paranoia. Index: Test/B02typeset.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/B02typeset.ztst,v retrieving revision 1.13 diff -u -r1.13 B02typeset.ztst --- Test/B02typeset.ztst 28 May 2007 22:57:42 -0000 1.13 +++ Test/B02typeset.ztst 30 Jul 2007 20:52:56 -0000 @@ -377,3 +377,14 @@ >integer local i >local tagged scalar >preserved + + export ENVFOO=bar + print ENVFOO in environment + env | grep '^ENVFOO' + unset ENVFOO + print ENVFOO no longer in environment + env | grep '^ENVFOO' +1:Adding and removing values to and from the environment +>ENVFOO in environment +>ENVFOO=bar +>ENVFOO no longer in environment -- Peter Stephenson <p.w.stephenson@ntlworld.com> Web page now at http://homepage.ntlworld.com/p.w.stephenson/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: putenv()/environ bug 2007-07-30 20:52 ` Peter Stephenson @ 2007-07-30 22:44 ` Sean C. Farley 2007-07-31 9:00 ` Peter Stephenson 0 siblings, 1 reply; 11+ messages in thread From: Sean C. Farley @ 2007-07-30 22:44 UTC (permalink / raw) To: Peter Stephenson; +Cc: Sean C. Farley, zsh-workers On Mon, 30 Jul 2007, Peter Stephenson wrote: > Paranoia. > > Index: Test/B02typeset.ztst > =================================================================== > RCS file: /cvsroot/zsh/zsh/Test/B02typeset.ztst,v > retrieving revision 1.13 > diff -u -r1.13 B02typeset.ztst > --- Test/B02typeset.ztst 28 May 2007 22:57:42 -0000 1.13 > +++ Test/B02typeset.ztst 30 Jul 2007 20:52:56 -0000 > @@ -377,3 +377,14 @@ > >integer local i > >local tagged scalar > >preserved > + > + export ENVFOO=bar > + print ENVFOO in environment > + env | grep '^ENVFOO' > + unset ENVFOO > + print ENVFOO no longer in environment > + env | grep '^ENVFOO' > +1:Adding and removing values to and from the environment > +>ENVFOO in environment > +>ENVFOO=bar > +>ENVFOO no longer in environment This test would work even before the patch. The issue only showed up if zsh was executed again. Example: zsh export FOO=BAR exec zsh unset FOO env | grep FOO echo $FOO Sean -- scf@FreeBSD.org ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: putenv()/environ bug 2007-07-30 22:44 ` Sean C. Farley @ 2007-07-31 9:00 ` Peter Stephenson 2007-07-31 18:09 ` Sean C. Farley 0 siblings, 1 reply; 11+ messages in thread From: Peter Stephenson @ 2007-07-31 9:00 UTC (permalink / raw) To: Sean C. Farley, zsh-workers On Mon, 30 Jul 2007 17:44:12 -0500 (CDT) "Sean C. Farley" <scf@FreeBSD.org> wrote: > The issue only showed up if zsh was executed again. > > Example: > zsh > export FOO=BAR > exec zsh > unset FOO > env | grep FOO > echo $FOO That's useful, I've added it. I've also made the code a bit more rational about always using setenv() and unsetenv() in pairs (this shouldn't change the effect except with a wonky library). Index: Src/exec.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/exec.c,v retrieving revision 1.120 diff -u -r1.120 exec.c --- Src/exec.c 30 Jul 2007 20:46:05 -0000 1.120 +++ Src/exec.c 31 Jul 2007 08:57:21 -0000 @@ -529,7 +529,7 @@ * for ARGV0: that's OK since we're about to exec or exit * on failure. */ -#ifdef HAVE_UNSETENV +#ifdef USE_SET_UNSET_ENV unsetenv("ARGV0"); #else delenvvalue(z - 6); Index: Src/params.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/params.c,v retrieving revision 1.134 diff -u -r1.134 params.c --- Src/params.c 30 Jul 2007 20:46:05 -0000 1.134 +++ Src/params.c 31 Jul 2007 08:57:21 -0000 @@ -610,7 +610,7 @@ createparamtable(void) { Param ip, pm; -#if !defined(HAVE_PUTENV) && !defined(HAVE_SETENV) +#if !defined(HAVE_PUTENV) && !defined(USE_SET_UNSET_ENV) char **new_environ; int envsize; #endif @@ -665,7 +665,7 @@ setsparam("LOGNAME", ztrdup((str = getlogin()) && *str ? str : cached_username)); -#if !defined(HAVE_PUTENV) && !defined(HAVE_SETENV) +#if !defined(HAVE_PUTENV) && !defined(USE_SET_UNSET_ENV) /* Copy the environment variables we are inheriting to dynamic * * memory, so we can do mallocs and frees on it. */ envsize = sizeof(char *)*(1 + arrlen(environ)); @@ -3855,7 +3855,7 @@ int zputenv(char *str) { -#ifdef HAVE_SETENV +#ifdef USE_SET_UNSET_ENV /* * If we are using unsetenv() to remove values from the * environment, which is the safe thing to do, we @@ -3906,7 +3906,7 @@ } /**/ -#ifndef HAVE_UNSETENV +#ifndef USE_SET_UNSET_ENV /**/ static int findenv(char *name, int *pos) @@ -3969,12 +3969,10 @@ addenv(Param pm, char *value) { char *newenv = 0; -#ifndef HAVE_UNSETENV +#ifndef USE_SET_UNSET_ENV char *oldenv = 0, *env = 0; int pos; -#endif -#ifndef HAVE_UNSETENV /* * First check if there is already an environment * variable matching string `name'. @@ -3989,7 +3987,7 @@ pm->env = NULL; return; } -#ifdef HAVE_UNSETENV +#ifdef USE_SET_UNSET_ENV /* * If we are using setenv/unsetenv to manage the environment, * we simply store the string we created in pm->env since @@ -4052,7 +4050,7 @@ * string. */ -#ifndef HAVE_UNSETENV +#ifndef USE_SET_UNSET_ENV /**/ void delenvvalue(char *x) @@ -4078,7 +4076,7 @@ void delenv(Param pm) { -#ifdef HAVE_UNSETENV +#ifdef USE_SET_UNSET_ENV unsetenv(pm->node.nam); zsfree(pm->env); #else Index: Src/system.h =================================================================== RCS file: /cvsroot/zsh/zsh/Src/system.h,v retrieving revision 1.45 diff -u -r1.45 system.h --- Src/system.h 30 Jul 2007 20:46:05 -0000 1.45 +++ Src/system.h 31 Jul 2007 08:57:22 -0000 @@ -697,8 +697,8 @@ * We always need setenv and unsetenv in pairs, because * we don't know how to do memory management on the values set. */ -#ifndef HAVE_UNSETENV -#undef HAVE_SETENV +#if defined(HAVE_SETENV) && defined(HAVE_UNSETENV) +# define USE_SET_UNSET_ENV #endif Index: Test/B02typeset.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/B02typeset.ztst,v retrieving revision 1.14 diff -u -r1.14 B02typeset.ztst --- Test/B02typeset.ztst 30 Jul 2007 20:55:41 -0000 1.14 +++ Test/B02typeset.ztst 31 Jul 2007 08:57:22 -0000 @@ -381,10 +381,27 @@ export ENVFOO=bar print ENVFOO in environment env | grep '^ENVFOO' + print Changing ENVFOO + ENVFOO="not bar any more" + env | grep '^ENVFOO' unset ENVFOO print ENVFOO no longer in environment env | grep '^ENVFOO' 1:Adding and removing values to and from the environment >ENVFOO in environment >ENVFOO=bar +>Changing ENVFOO +>ENVFOO=not bar any more >ENVFOO no longer in environment + + (export FOOENV=BAR + env | grep '^FOOENV' + print Exec + exec $ZTST_testdir/../Src/zsh -c ' + print Unset + unset FOOENV + env | grep "^FOOENV"') +1:Can unset environment variables after exec +>FOOENV=BAR +>Exec +>Unset . ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: putenv()/environ bug 2007-07-31 9:00 ` Peter Stephenson @ 2007-07-31 18:09 ` Sean C. Farley 0 siblings, 0 replies; 11+ messages in thread From: Sean C. Farley @ 2007-07-31 18:09 UTC (permalink / raw) To: Peter Stephenson; +Cc: Sean C. Farley, zsh-workers On Tue, 31 Jul 2007, Peter Stephenson wrote: > On Mon, 30 Jul 2007 17:44:12 -0500 (CDT) > "Sean C. Farley" <scf@FreeBSD.org> wrote: >> The issue only showed up if zsh was executed again. *snip test case* > That's useful, I've added it. I've also made the code a bit more > rational about always using setenv() and unsetenv() in pairs (this > shouldn't change the effect except with a wonky library). *snip patch* This new patch works too. Thank you. Note to FreeBSD 7-CURRENT users: I will submit a FreeBSD PR, so it will be patched in the port. Sean -- scf@FreeBSD.org ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: putenv()/environ bug 2007-07-30 20:39 ` Peter Stephenson 2007-07-30 20:52 ` Peter Stephenson @ 2007-07-30 22:39 ` Sean C. Farley 1 sibling, 0 replies; 11+ messages in thread From: Sean C. Farley @ 2007-07-30 22:39 UTC (permalink / raw) To: Peter Stephenson; +Cc: Sean C. Farley, zsh-workers On Mon, 30 Jul 2007, Peter Stephenson wrote: > On Sun, 29 Jul 2007 14:08:42 -0500 (CDT) > "Sean C. Farley" <scf@FreeBSD.org> wrote: >> On Sat, 28 Jul 2007, Andrey Borzenkov wrote: >>> What about - check if (un-)setenv is available and use them. On >>> legacy systems use existing implementation. This probably will mean >>> we will be using native utilities everywhere on modern systems. >> >> That would work for me. If anyone would like me to test any patches, >> I will. > > This sounds the best, and it's working on Fedora 7. > > I've tested if both setenv() and unsetenv() are available and if they > are we don't do any management of the environment ourselves, so the > hack in addenv() doesn't occur... I hope that works, otherwise the > implementation of the library is fairly well broken, the whole point > of setenv/unsetenv being to be able to do this in a standard fashion. > > The only references to environ we make with this code are to read it > at the start to initialise internval variables and to pass it to > execs. This seems to be sanctioned by POSIX. > > As a TODO indicates, this means we don't actually need to store an > "env" string in the parameter structure, just a flag saying we've > stuck the value in the environment. That can change when this stuff > is clearly working. This change fixes the issue concerning the *env() functions in FreeBSD 7-CURRENT. It works also with 6-STABLE. Thank you. Sean P.S. I am not subscribed; please keep me in the Cc. -- scf@FreeBSD.org ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2007-07-31 18:10 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-07-25 15:09 putenv()/environ bug Sean C. Farley 2007-07-25 20:53 ` Peter Stephenson 2007-07-26 0:14 ` Sean C. Farley 2007-07-28 18:46 ` Andrey Borzenkov 2007-07-29 19:08 ` Sean C. Farley 2007-07-30 20:39 ` Peter Stephenson 2007-07-30 20:52 ` Peter Stephenson 2007-07-30 22:44 ` Sean C. Farley 2007-07-31 9:00 ` Peter Stephenson 2007-07-31 18:09 ` Sean C. Farley 2007-07-30 22:39 ` Sean C. Farley
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).