Re: [bug] shwordsplit not working on $@ when $# > 1

[Bart Schaefer <schaefer@brasslantern.com>]

On Aug 8,  7:27pm, Peter Stephenson wrote:
} Subject: Re: [bug] shwordsplit not working on $@ when $# > 1
}
} The variable isarr from the value entry v->isarr is negative, so we
} don't go into the loop that does joining and splitting as that
} explicitly asks for (isarr >= 0).  (It was 0 for a scalar, hence the
} first case above.)

It's not really a loop; it's just the "if" block at subst.c:3457.
This block first joins all the array elements on the first character
of $IFS, and then splits the resulting string on spaces.

The problem is that in this case we want split but in the case of
users/15439 we do NOT want join.  The patch called out by Jeremie
prevents the joining, but does so by bypassing the splitting; it
first sets nojoin nonzero by examining IFS, and then sets isarr = -1
when nojoin is true (i.e., the negative isarr you saw is not coming
from v->isarr, so the value of SCANPM_ISVAR_AT is a red herring).

The underlying assumption that it works to first join on $IFS and
then split again to get the array leads to the error, because when
$IFS is empty there won't be anything to split on, but when there is
no join the new array must be built by splitting every element of
the value array (and there is currently no code that does that).

I *think* we can untangle this as follows, but then again I thought
I had untangled in in workers/29313, too.  This relies on the idea
that if we already have an array when nojoin, then we're not going
to split it again, which seems dubious somehow if there is explicit
use of the (s:-:) flag.  It may be that we really do need to write
a loop over the existing elements when isarr is true there.

Existing tests still pass, but then they always did, this needs a
new one.  Holding off until we think of other edge cases.

diff --git a/Src/subst.c b/Src/subst.c
index e3af156..6dd4608 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -3454,13 +3454,13 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
      * exception is that ${name:-word} and ${name:+word} will have already
      * done any requested splitting of the word value with quoting preserved.
      */
-    if (ssub || (spbreak && isarr >= 0) || spsep || sep) {
-	if (isarr) {
+    if (ssub || spbreak || spsep || sep) {
+	if (isarr && !nojoin) {
 	    val = sepjoin(aval, sep, 1);
 	    isarr = 0;
 	    ms_flags = 0;
 	}
-	if (!ssub && (spbreak || spsep)) {
+	if (!ssub && (spbreak || spsep) && !isarr) {
 	    aval = sepsplit(val, spsep, 0, 1);
 	    if (!aval || !aval[0])
 		val = dupstring("");
@@ -3527,7 +3527,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
 	}
 	/*
 	 * TODO:  It would be really quite nice to abstract the
-	 * isarr and !issarr code into a function which gets
+	 * isarr and !isarr code into a function which gets
 	 * passed a pointer to a function with the effect of
 	 * the promptexpand bit.  Then we could use this for
 	 * a lot of stuff and bury val/aval/isarr inside a structure