From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4773 invoked from network); 30 Jul 2007 20:41:20 -0000 X-Spam-Checker-Version: SpamAssassin 3.2.1 (2007-05-02) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.2.1 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by ns1.primenet.com.au with SMTP; 30 Jul 2007 20:41:20 -0000 Received-SPF: none (ns1.primenet.com.au: domain at sunsite.dk does not designate permitted sender hosts) Received: (qmail 51168 invoked from network); 30 Jul 2007 20:41:14 -0000 Received: from sunsite.dk (130.225.247.90) by a.mx.sunsite.dk with SMTP; 30 Jul 2007 20:41:14 -0000 Received: (qmail 23020 invoked by alias); 30 Jul 2007 20:41:11 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 23725 Received: (qmail 23010 invoked from network); 30 Jul 2007 20:41:10 -0000 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by sunsite.dk with SMTP; 30 Jul 2007 20:41:10 -0000 Received: (qmail 50832 invoked from network); 30 Jul 2007 20:41:10 -0000 Received: from mtaout01-winn.ispmail.ntl.com (81.103.221.47) by a.mx.sunsite.dk with SMTP; 30 Jul 2007 20:41:02 -0000 Received: from aamtaout01-winn.ispmail.ntl.com ([81.103.221.35]) by mtaout01-winn.ispmail.ntl.com with ESMTP id <20070730204100.UVJM1783.mtaout01-winn.ispmail.ntl.com@aamtaout01-winn.ispmail.ntl.com> for ; Mon, 30 Jul 2007 21:41:00 +0100 Received: from pws-pc.ntlworld.com ([81.107.45.67]) by aamtaout01-winn.ispmail.ntl.com with SMTP id <20070730204100.MGQC219.aamtaout01-winn.ispmail.ntl.com@pws-pc.ntlworld.com> for ; Mon, 30 Jul 2007 21:41:00 +0100 Date: Mon, 30 Jul 2007 21:39:34 +0100 From: Peter Stephenson To: zsh-workers@sunsite.dk Subject: Re: putenv()/environ bug Message-Id: <20070730213934.aefff281.p.w.stephenson@ntlworld.com> In-Reply-To: <20070729140248.D2588@thor.farley.org> References: <20070725093254.T20275@thor.farley.org> <20070725215321.00e3b110.p.w.stephenson@ntlworld.com> <20070725184302.S23862@thor.farley.org> <200707282246.16663.arvidjaar@newmail.ru> <20070729140248.D2588@thor.farley.org> X-Mailer: Sylpheed 2.3.1 (GTK+ 2.10.13; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Sun, 29 Jul 2007 14:08:42 -0500 (CDT) "Sean C. Farley" 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 Web page now at http://homepage.ntlworld.com/p.w.stephenson/