23725: use setenv()/unsetenv() for environment memory management

ChangeLog

@@ -1,3 +1,9 @@
+2007-07-30  Peter Stephenson  <p.w.stephenson@ntlworld.com>
+
+	* 23725: configure.ac, Src/exec.c, Src/params.c, Src/system.h:
+	use setenv() and unsetenv() for memory management of the
+	environment where possible.
+
 2007-07-29  Clint Adams  <clint@zsh.org>
 
 	* 23720: Completion/Unix/Command/_dvi: handle arguments to dvips -P

Src/exec.c

@@ -524,7 +524,16 @@ execute(LinkList args, int flags, int defpath)
      * 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.         */

Src/params.c

@@ -610,7 +610,7 @@ void
 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 @@ createparamtable(void)
 
     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 @@ arrfixenv(char *s, char **t)
 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,8 +3902,11 @@ zputenv(char *str)
     }
     return 0;
 #endif
+#endif
 }
 
+/**/
+#ifndef HAVE_UNSETENV
 /**/
 static int
 findenv(char *name, int *pos)
@@ -3899,6 +3926,8 @@ findenv(char *name, int *pos)
     
     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 @@ copyenvstr(char *s, char *value, int flags)
 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 @@ addenv(Param pm, char *value)
 	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 @@ addenv(Param pm, char *value)
 
     DPUTS(1, "addenv should never reach the end");
     pm->env = NULL;
+#endif
 }
 
 
@@ -4003,6 +4052,7 @@ mkenvstr(char *name, char *value, int flags)
  * string.                                         */
 
 
+#ifndef HAVE_UNSETENV
 /**/
 void
 delenvvalue(char *x)
@@ -4018,6 +4068,8 @@ delenvvalue(char *x)
     }
     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 @@ delenvvalue(char *x)
 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

Src/system.h

@@ -693,6 +693,15 @@ struct timezone {
 
 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

configure.ac

@@ -1126,7 +1126,7 @@ AC_CHECK_FUNCS(strftime strptime mktime timelocal \
 	       setlocale \
 	       uname \
 	       signgam \
-	       putenv getenv \
+	       putenv getenv setenv unsetenv xw\
 	       brk sbrk \
 	       pathconf sysconf \
 	       tgetent tigetflag tigetnum tigetstr setupterm \