Re: [bug] sh: tilde expansion after field splitting

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Fri, 13 Oct 2017 11:43:17 +0200
Martijn Dekker <martijn@xxxxxxxx> wrote:
> Op 08-10-17 om 09:53 schreef Bart Schaefer:
> > On Oct 5, 12:20am, Martijn Dekker wrote:
> > } Subject: [bug] sh: tilde expansion after field splitting
> > }
> > } POSIX says tilde expansion should be done before parameter expansion [...]
> > } zsh did this correctly up to version 5.0.8; as of 5.1, it appears to do
> > } tilde expansion *after* field splitting, and only from the second field on.
> > 
> > The patch below fixes this, I believe.  Several comments:
> 
> This introduces a bug with "$@", $@ and $* expansion:
> 
> $ Src/zsh -o SHFILEEXPANSION -c 'set "a" "b"; printf "[%s]\n" "$@$@"'
> [a]
> [b$@]
> 
> $ Src/zsh -o SHFILEEXPANSION -c 'set "a" "b"; printf "[%s]\n" $@$@'
> [a]
> [b$@]
> 
> $ zsh -o SHFILEEXPANSION -c 'set "a" "b"; printf "[%s]\n" $*$*'
> zsh:1: no matches found: b$*

Would it be easier to do something like this?  I don't think efficiency
is much of an issue here.

pws

diff --git a/Src/subst.c b/Src/subst.c
index 8c290cc..d027e3d 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -108,7 +108,6 @@ prefork(LinkList list, int flags, int *ret_flags)
     queue_signals();
     node = firstnode(list);
     while (node) {
-	LinkNode nextnode = 0;
 	if ((flags & (PREFORK_SINGLE|PREFORK_ASSIGN)) == PREFORK_ASSIGN &&
 	    (insnode = keyvalpairelement(list, node))) {
 	    node = insnode;
@@ -137,23 +136,31 @@ prefork(LinkList list, int flags, int *ret_flags)
 	     * testing if cptr changed...
 	     */
 	    setdata(node, cptr);
-	    /*
-	     * Advance now because we must not expand filenames again
-	     * after string substitution (which may insert new nodes).
-	     */
-	    nextnode = node;
-	    incnode(nextnode);
-	}
-	if (!(node = stringsubst(list, node,
-				 flags & ~(PREFORK_TYPESET|PREFORK_ASSIGN),
-				 ret_flags, asssub))) {
-	    unqueue_signals();
-	    return;
 	}
-	if (isset(SHFILEEXPANSION))
-	    node = nextnode;
 	else
-	    incnode(node);
+	{
+	    if (!(node = stringsubst(list, node,
+				     flags & ~(PREFORK_TYPESET|PREFORK_ASSIGN),
+				     ret_flags, asssub))) {
+		unqueue_signals();
+		return;
+	    }
+	}
+	incnode(node);
+    }
+    if (isset(SHFILEEXPANSION)) {
+	/*
+	 * stringsubst() may insert new nodes, so doesn't work
+	 * well in the same loop as file expansion.
+	 */
+	for (node = firstnode(list); node; incnode(node)) {
+	    if (!(node = stringsubst(list, node,
+				     flags & ~(PREFORK_TYPESET|PREFORK_ASSIGN),
+				     ret_flags, asssub))) {
+		unqueue_signals();
+		return;
+	    }
+	}
     }
     for (node = firstnode(list); node; incnode(node)) {
 	if (node == stop)
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 6929f51..0f6bb34 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -1038,6 +1038,16 @@ F:Regression test for workers/41811
 >~/one
 >~/two
 
+  (
+    setopt shfileexpansion
+    set -- also appearing
+    print -l $*$*
+  )
+0:SH_FILE_EXPANSION interaction with inserting nodes from parameters
+>also
+>appearingalso
+>appearing
+
   testpat() {
     if [[ $1 = ${~2} ]]; then print $1 $2 yes; else print $1 $2 no; fi
   }