28220 plus some comments: fix "HELLO=$HELLO shellfunc"

ChangeLog

@@ -1,5 +1,10 @@
 2010-08-31  Peter Stephenson  <p.w.stephenson@ntlworld.com>
 
+	* 28220: Src/exec.c (plus comments), Src/params.c,
+	Test/A06assign.ztst: "HELLO=$HELLO shellfunc" failed because
+	we removed HELLO from the parameter table to save it.  Copy it
+	instead.
+
 	* Mikael: 28202: Src/Zle/complist.c: need line unmetafied for
 	reversemenucomplete().
 
@@ -13576,5 +13581,5 @@
 
 *****************************************************
 * This is used by the shell to define $ZSH_PATCHLEVEL
-* $Revision: 1.5065 $
+* $Revision: 1.5066 $
 *****************************************************

Src/exec.c

@@ -3313,13 +3313,32 @@ 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))) {
+	    Param tpm;
 	    if (pm->env)
 		delenv(pm);
 	    if (!(pm->node.flags & PM_SPECIAL)) {
-		paramtab->removenode(paramtab, s);
+		/*
+		 * We used to remove ordinary parameters from the
+		 * table, but that meant "HELLO=$HELLO shellfunc"
+		 * failed because the expansion of $HELLO hasn't
+		 * been done at this point.  Instead, copy the
+		 * parameter:  in this case, we'll insert the
+		 * copied parameter straight back into the parameter
+		 * table so we wan't to be sure everything is
+		 * properly set up and in permanent memory.
+		 */
+		tpm = (Param) zshcalloc(sizeof *tpm);
+		tpm->node.nam = ztrdup(pm->node.nam);
+		copyparam(tpm, pm, 0);
+		pm = tpm;
 	    } else if (!(pm->node.flags & PM_READONLY) &&
 		       (unset(RESTRICTED) || !(pm->node.flags & PM_RESTRICTED))) {
-		Param tpm = (Param) hcalloc(sizeof *tpm);
+		/*
+		 * In this case we're just saving parts of
+		 * the parameter in a tempory, so use heap allocation
+		 * and don't bother copying every detail.
+		 */
+		tpm = (Param) hcalloc(sizeof *tpm);
 		tpm->node.nam = pm->node.nam;
 		copyparam(tpm, pm, 1);
 		pm = tpm;
@@ -3383,8 +3402,9 @@ restore_params(LinkList restorelist, LinkList removelist)
 		    break;
 		}
 		pm = tpm;
-	    } else
+	    } else {
 		paramtab->addnode(paramtab, pm->node.nam, pm);
+	    }
 	    if ((pm->node.flags & PM_EXPORTED) && ((s = getsparam(pm->node.nam))))
 		addenv(pm, s);
 	}

Src/params.c

@@ -927,11 +927,17 @@ createspecialhash(char *name, GetNodeFunc get, ScanTabFunc scan, int flags)
 }
 
 
-/* Copy a parameter */
+/*
+ * Copy a parameter
+ *
+ * If fakecopy is set, we are just saving the details of a special
+ * parameter.  Otherwise, the result will be used as a real parameter
+ * and we need to do more work.
+ */
 
 /**/
 void
-copyparam(Param tpm, Param pm, int toplevel)
+copyparam(Param tpm, Param pm, int fakecopy)
 {
     /*
      * Note that tpm, into which we're copying, may not be in permanent
@@ -942,7 +948,8 @@ copyparam(Param tpm, Param pm, int toplevel)
     tpm->node.flags = pm->node.flags;
     tpm->base = pm->base;
     tpm->width = pm->width;
-    if (!toplevel)
+    tpm->level = pm->level;
+    if (!fakecopy)
 	tpm->node.flags &= ~PM_SPECIAL;
     switch (PM_TYPE(pm->node.flags)) {
     case PM_SCALAR:
@@ -963,13 +970,15 @@ copyparam(Param tpm, Param pm, int toplevel)
 	break;
     }
     /*
-     * If called from inside an associative array, that array is later going
-     * to be passed as a real parameter, so we need the gets and sets
-     * functions to be useful.  However, the saved associated array is
-     * not itself special, so we just use the standard ones.
-     * This is also why we switch off PM_SPECIAL.
+     * If the value is going to be passed as a real parameter (e.g. this is
+     * called from inside an associative array), we need the gets and sets
+     * functions to be useful.
+     *
+     * In this case we assume the the saved parameter is not itself special,
+     * so we just use the standard functions.  This is also why we switch off
+     * PM_SPECIAL.
      */
-    if (!toplevel)
+    if (!fakecopy)
 	assigngetset(tpm);
 }
 

Test/A06assign.ztst

@@ -277,3 +277,18 @@
 >
 >
 >
+
+  call() { print $HELLO; }
+  export HELLO=world
+  call
+  HELLO=universe call
+  call
+  HELLO=${HELLO}liness call
+  call
+  unset HELLO
+0:save and restore when using original value in temporary
+>world
+>universe
+>world
+>worldliness
+>world