20141: Crash restoring exported special parameters

4 files changed, 66 insertions, 63 deletions

diff --git a/ChangeLog b/ChangeLog
index e3e86d35e..3aff2e939 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2004-07-07  Peter Stephenson  <pws@csr.com>
+
+	* 20141: Src/builtin.c, Src/exec.c, Src/params.c:  Crash
+	after restoring exported special parameters.  The
+	answer was `pm = tpm' at exec.c:2590.
+
 2004-07-02  Peter Stephenson  <pws@csr.com>
 
 	* 20127: Doc/Zsh/zle.yo, Src/Zle/zle_main.c,
diff --git a/Src/builtin.c b/Src/builtin.c
index 81bba7d4f..294b405b3 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -750,15 +750,11 @@ set_pwd_env(void)
     setsparam("OLDPWD", ztrdup(oldpwd));
 
     pm = (Param) paramtab->getnode(paramtab, "PWD");
-    if (!(pm->flags & PM_EXPORTED)) {
-	pm->flags |= PM_EXPORTED;
-	pm->env = addenv("PWD", pwd, pm->flags);
-    }
+    if (!(pm->flags & PM_EXPORTED))
+	addenv(pm, pwd);
     pm = (Param) paramtab->getnode(paramtab, "OLDPWD");
-    if (!(pm->flags & PM_EXPORTED)) {
-	pm->flags |= PM_EXPORTED;
-	pm->env = addenv("OLDPWD", oldpwd, pm->flags);
-    }
+    if (!(pm->flags & PM_EXPORTED))
+	addenv(pm, oldpwd);
 }
 
 /* set if we are resolving links to their true paths */
@@ -1883,11 +1879,9 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	if (!(pm->flags & (PM_ARRAY|PM_HASHED))) {
 	    if (pm->flags & PM_EXPORTED) {
 		if (!(pm->flags & PM_UNSET) && !pm->env && !value)
-		    pm->env = addenv(pname, getsparam(pname), pm->flags);
-	    } else if (pm->env && !(pm->flags & PM_HASHELEM)) {
-		delenv(pm->env);
-		pm->env = NULL;
-	    }
+		    addenv(pm, getsparam(pname));
+	    } else if (pm->env && !(pm->flags & PM_HASHELEM))
+		delenv(pm);
 	    if (value && !(pm = setsparam(pname, ztrdup(value))))
 		return NULL;
 	} else if (value) {
@@ -1938,7 +1932,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	 * Maybe it would be easier to create a new struct but copy
 	 * the get/set methods.
 	 */
-	tpm = (Param) zalloc(sizeof *tpm);
+	tpm = (Param) zshcalloc(sizeof *tpm);
 
 	tpm->nam = pm->nam;
 	if (pm->ename &&
@@ -1962,10 +1956,9 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	tpm->old = pm->old;
 	tpm->level = pm->level;
 	tpm->ct = pm->ct;
-	if (pm->env) {
-	    delenv(pm->env);
-	}
-	tpm->env = pm->env = NULL;
+	if (pm->env)
+	    delenv(pm);
+	tpm->env = NULL;
 
 	pm->old = tpm;
 	/*
diff --git a/Src/exec.c b/Src/exec.c
index 90e75db64..83f397335 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -483,7 +483,7 @@ execute(UNUSED(Cmdnam cmdname), int dash, int defpath)
      * that as argv[0] for this external command       */
     if (unset(RESTRICTED) && (z = zgetenv("ARGV0"))) {
 	setdata(firstnode(args), (void *) ztrdup(z));
-	delenv(z - 6);
+	delenvvalue(z - 6);
     } else if (dash) {
     /* Else if the pre-command `-' was given, we add `-' *
      * to the front of argv[0] for this command.         */
@@ -2520,15 +2520,13 @@ save_params(Estate state, Wordcode pc, LinkList *restore_p, LinkList *remove_p)
     while (wc_code(ac = *pc) == WC_ASSIGN) {
 	s = ecrawstr(state->prog, pc + 1, NULL);
 	if ((pm = (Param) paramtab->getnode(paramtab, s))) {
-	    if (pm->env) {
-		delenv(pm->env);
-		pm->env = NULL;
-	    }
+	    if (pm->env)
+		delenv(pm);
 	    if (!(pm->flags & PM_SPECIAL)) {
 		paramtab->removenode(paramtab, s);
 	    } else if (!(pm->flags & PM_READONLY) &&
 		       (unset(RESTRICTED) || !(pm->flags & PM_RESTRICTED))) {
-		Param tpm = (Param) zhalloc(sizeof *tpm);
+		Param tpm = (Param) hcalloc(sizeof *tpm);
 		tpm->nam = pm->nam;
 		copyparam(tpm, pm, 1);
 		pm = tpm;
@@ -2589,10 +2587,11 @@ restore_params(LinkList restorelist, LinkList removelist)
 		    tpm->sets.hfn(tpm, pm->u.hash);
 		    break;
 		}
+		pm = tpm;
 	    } else
 		paramtab->addnode(paramtab, pm->nam, pm);
 	    if ((pm->flags & PM_EXPORTED) && ((s = getsparam(pm->nam))))
-		pm->env = addenv(pm->nam, s, pm->flags);
+		addenv(pm, s);
 	}
     }
 }
diff --git a/Src/params.c b/Src/params.c
index 9d9d39778..4c2a17ee8 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -587,20 +587,15 @@ createparamtable(void)
     opts[ALLEXPORT] = oae;
 
     pm = (Param) paramtab->getnode(paramtab, "HOME");
-    if (!(pm->flags & PM_EXPORTED)) {
-	pm->flags |= PM_EXPORTED;
-	pm->env = addenv("HOME", home, pm->flags);
-    }
+    if (!(pm->flags & PM_EXPORTED))
+	addenv(pm, home);
     pm = (Param) paramtab->getnode(paramtab, "LOGNAME");
-    if (!(pm->flags & PM_EXPORTED)) {
-	pm->flags |= PM_EXPORTED;
-	pm->env = addenv("LOGNAME", pm->u.str, pm->flags);
-    }
-    pm = (Param) paramtab->getnode(paramtab, "SHLVL");
     if (!(pm->flags & PM_EXPORTED))
-	pm->flags |= PM_EXPORTED;
+	addenv(pm, pm->u.str);
+    pm = (Param) paramtab->getnode(paramtab, "SHLVL");
     sprintf(buf, "%d", (int)++shlvl);
-    pm->env = addenv("SHLVL", buf, pm->flags);
+    if (!(pm->flags & PM_EXPORTED))
+	addenv(pm, buf);
 
     /* Add the standard non-special parameters */
     set_pwd_env();
@@ -712,10 +707,8 @@ createparam(char *name, int flags)
 		 * needed to avoid freeing oldpm, but we do take it
 		 * out of the environment when it's hidden.
 		 */
-		if (oldpm->env) {
-		    delenv(oldpm->env);
-		    oldpm->env = NULL;
-		}
+		if (oldpm->env)
+		    delenv(oldpm);
 		paramtab->removenode(paramtab, name);
 	    }
 	    paramtab->addnode(paramtab, ztrdup(name), pm);
@@ -1585,8 +1578,7 @@ export_param(Param pm)
     else
 	val = pm->gets.cfn(pm);
 
-    pm->flags |= PM_EXPORTED;
-    pm->env = addenv(pm->nam, val, pm->flags);
+    addenv(pm, val);
 }
 
 /**/
@@ -2240,10 +2232,8 @@ unsetparam_pm(Param pm, int altflag, int exp)
 	return 1;
     }
     pm->unsetfn(pm, exp);
-    if ((pm->flags & PM_EXPORTED) && pm->env) {
-	delenv(pm->env);
-	pm->env = NULL;
-    }
+    if (pm->env)
+	delenv(pm);
 
     /* remove it under its alternate name if necessary */
     if (pm->ename && !altflag) {
@@ -3296,7 +3286,7 @@ arrfixenv(char *s, char **t)
     else
 	joinchar = ':';
 
-    pm->env = addenv(s, t ? zjoin(t, joinchar, 1) : "", pm->flags);
+    addenv(pm, t ? zjoin(t, joinchar, 1) : "");
 }
 
 
@@ -3385,8 +3375,8 @@ copyenvstr(char *s, char *value, int flags)
 }
 
 /**/
-char *
-addenv(char *name, char *value, int flags)
+void
+addenv(Param pm, char *value)
 {
     char *oldenv = 0, *newenv = 0, *env = 0;
     int pos;
@@ -3394,13 +3384,14 @@ addenv(char *name, char *value, int flags)
     /* First check if there is already an environment *
      * variable matching string `name'. If not, and   *
      * we are not requested to add new, return        */
-    if (findenv(name, &pos))
+    if (findenv(pm->nam, &pos))
 	oldenv = environ[pos];
 
-     newenv = mkenvstr(name, value, flags);
+     newenv = mkenvstr(pm->nam, value, pm->flags);
      if (zputenv(newenv)) {
         zsfree(newenv);
-	return NULL;
+	pm->env = NULL;
+	return;
     }
     /*
      * Under Cygwin we must use putenv() to maintain consistency.
@@ -3408,16 +3399,19 @@ addenv(char *name, char *value, int flags)
      * silently reuse existing environment string. This tries to
      * check for both cases
      */
-    if (findenv(name, &pos)) {
+    if (findenv(pm->nam, &pos)) {
 	env = environ[pos];
 	if (env != oldenv)
 	    zsfree(oldenv);
 	if (env != newenv)
 	    zsfree(newenv);
-	return env;
+	pm->flags |= PM_EXPORTED;
+	pm->env = env;
+	return;
     }
 
-    return NULL; /* Cannot happen */
+    DPUTS(1, "addenv should never reach the end");
+    pm->env = NULL;
 }
 
 
@@ -3448,12 +3442,9 @@ mkenvstr(char *name, char *value, int flags)
  * string.                                         */
 
 
-/* Delete a pointer from the list of pointers to environment *
- * variables by shifting all the other pointers up one slot. */
-
 /**/
 void
-delenv(char *x)
+delenvvalue(char *x)
 {
     char **ep;
 
@@ -3467,6 +3458,22 @@ delenv(char *x)
     zsfree(x);
 }
 
+/* Delete a pointer from the list of pointers to environment *
+ * variables by shifting all the other pointers up one slot. */
+
+/**/
+void
+delenv(Param pm)
+{
+    delenvvalue(pm->env);
+    pm->env = NULL;
+    /*
+     * Note we don't remove PM_EXPORT from the flags.  This
+     * may be asking for trouble but we need to know later
+     * if we restore this parameter to its old value.
+     */
+}
+
 /**/
 mod_export void
 convbase(char *s, zlong v, int base)
@@ -3625,10 +3632,8 @@ scanendscope(HashNode hn, UNUSED(int flags))
 	    pm->flags = (tpm->flags & ~PM_NORESTORE);
 	    pm->level = tpm->level;
 	    pm->ct = tpm->ct;
-	    if (pm->env) {
-		delenv(pm->env);
-	    }
-	    pm->env = NULL;
+	    if (pm->env)
+		delenv(pm);
 
 	    if (!(tpm->flags & PM_NORESTORE))
 		switch (PM_TYPE(pm->flags)) {