Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: Command substitution parsing issues (not really Re: completion)



On Sun, 18 Jan 2015 17:31:57 +0000
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
> % alias foo='echo poop'
> % echo ${(e):-'$(foo)'}
> ÿÿÿÿÿÿÿÿ$(foo)
> 
> This comes from the call from subst_parse_str() to parsestr() and
> parsestrnoerr(), which make unwarranted and undocumented assumptions
> about the way the lexer works, in particular that if it passes in a
> variable s then any parsing happens within that string --- a pretty
> stupid assumption.  The interface needs fixing up, which
> ought to be easy enough just by ensuring parsestrnoerr() passes back a
> reference to tokstr --- however, I see parsestr is called all over
> the place so I'm not going to have time to look now.

Changing the interface looks like it works OK.

> We also need to ensure and document that the string parsed in comes from
> the heap --- I think that's already a requirement, just nobody got
> around to writing it down.

And this was broken, too.  If the token string had needed resizing the
whole thing would have fallen over in a big heap.  The hope was that
would be avoided by using the input string as the output string ---
that's why it was duplicated to go into the input buffer.  However,
there was no contract with the lexer that it had to work this way.  I've
changed to use dupstring() where necessary and noted in the comment.

> In fact parsestrnoerr() appears to be doing
> all sorts of hair-raising things without documentation --- why set the
> input string and then immediately after set the lexical buffer to the
> original string?

Just explained above.

pws


diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c
index 43dd4e2..189582d 100644
--- a/Src/Zle/compctl.c
+++ b/Src/Zle/compctl.c
@@ -3853,7 +3853,7 @@ makecomplistflags(Compctl cc, char *s, int incmd, int compadd)
 	    yaptr = get_user_var(uv);
 	if ((tt = cc->explain)) {
 	    tt = dupstring(tt);
-	    if ((cc->mask & CC_EXPANDEXPL) && !parsestr(tt)) {
+	    if ((cc->mask & CC_EXPANDEXPL) && !parsestr(&tt)) {
 		singsub(&tt);
 		untokenize(tt);
 	    }
@@ -3873,7 +3873,7 @@ makecomplistflags(Compctl cc, char *s, int incmd, int compadd)
 	}
     } else if ((tt = cc->explain)) {
 	tt = dupstring(tt);
-	if ((cc->mask & CC_EXPANDEXPL) && !parsestr(tt)) {
+	if ((cc->mask & CC_EXPANDEXPL) && !parsestr(&tt)) {
 	    singsub(&tt);
 	    untokenize(tt);
 	}
diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c
index dbef7f8..9f383f4 100644
--- a/Src/Zle/compresult.c
+++ b/Src/Zle/compresult.c
@@ -1090,7 +1090,8 @@ do_single(Cmatch m)
 		    }
 		    if (tryit) {
 			noerrs = 1;
-			parsestr(p);
+			p = dupstring(p);
+			parsestr(&p);
 			singsub(&p);
 			errflag &= ~ERRFLAG_ERROR;
 			noerrs = ne;
diff --git a/Src/exec.c b/Src/exec.c
index 7b64951..f42fb2b 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3772,19 +3772,20 @@ gethere(char **strp, int typ)
 	*bptr++ = '\n';
     }
     *t = '\0';
+    s = buf;
+    buf = dupstring(buf);
+    zfree(s, bsiz);
     if (!qt) {
 	int ef = errflag;
 
-	parsestr(buf);
+	parsestr(&buf);
 
 	if (!errflag) {
 	    /* Retain any user interrupt error */
 	    errflag = ef | (errflag & ERRFLAG_INT);
 	}
     }
-    s = dupstring(buf);
-    zfree(buf, bsiz);
-    return s;
+    return buf;
 }
 
 /* open here string fd */
diff --git a/Src/init.c b/Src/init.c
index e7d86fe..3e41fb9 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -1197,10 +1197,13 @@ run_init_scripts(void)
 	    if (islogin)
 		sourcehome(".profile");
 	    noerrs = 2;
-	    if (s && !parsestr(s)) {
-		singsub(&s);
-		noerrs = 0;
-		source(s);
+	    if (s) {
+		s = dupstring(s);
+		if (!parsestr(&s)) {
+		    singsub(&s);
+		    noerrs = 0;
+		    source(s);
+		}
 	    }
 	    noerrs = 0;
 	} else
diff --git a/Src/lex.c b/Src/lex.c
index e4dfdfa..0270ec9 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -1503,17 +1503,26 @@ dquote_parse(char endchar, int sub)
     return err;
 }
 
-/* Tokenize a string given in s. Parsing is done as in double *
- * quotes.  This is usually called before singsub().          */
+/*
+ * Tokenize a string given in s. Parsing is done as in double
+ * quotes.  This is usually called before singsub().
+ *
+ * parsestr() is noisier, reporting an error if the parse failed.
+ *
+ * On entry, *s must point to a string allocated from the stack of
+ * exactly the right length, i.e. strlen(*s), as the string
+ * is used as the lexical token string whose memory management
+ * demands this.
+ */
 
 /**/
 mod_export int
-parsestr(char *s)
+parsestr(char **s)
 {
     int err;
 
     if ((err = parsestrnoerr(s))) {
-	untokenize(s);
+	untokenize(*s);
 	if (err > 32 && err < 127)
 	    zerr("parse error near `%c'", err);
 	else
@@ -1524,18 +1533,20 @@ parsestr(char *s)
 
 /**/
 mod_export int
-parsestrnoerr(char *s)
+parsestrnoerr(char **s)
 {
-    int l = strlen(s), err;
+    int l = strlen(*s), err;
 
     zcontext_save();
-    untokenize(s);
-    inpush(dupstring(s), 0, NULL);
+    untokenize(*s);
+    inpush(dupstring(*s), 0, NULL);
     strinbeg(0);
     lexbuf.len = 0;
-    lexbuf.ptr = tokstr = s;
+    lexbuf.ptr = tokstr = *s;
     lexbuf.siz = l + 1;
     err = dquote_parse('\0', 1);
+    if (tokstr)
+	*s = tokstr;
     *lexbuf.ptr = '\0';
     strinend();
     inpop();
diff --git a/Src/params.c b/Src/params.c
index b8e0c42..64c78bd 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -1260,7 +1260,8 @@ getarg(char **str, int *inv, Value v, int a2, zlong *w,
     if (ishash && (keymatch || !rev))
 	remnulargs(s);
     if (needtok) {
-	if (parsestr(s))
+	s = dupstring(s);
+	if (parsestr(&s))
 	    return 0;
 	singsub(&s);
     } else if (rev)
diff --git a/Src/prompt.c b/Src/prompt.c
index 3552575..ffc1d0d 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -183,7 +183,7 @@ promptexpand(char *s, int ns, char *rs, char *Rs, unsigned int *txtchangep)
 	int oldval = lastval;
 
 	s = dupstring(s);
-	if (!parsestr(s))
+	if (!parsestr(&s))
 	    singsub(&s);
 	/*
 	 * We don't need the special Nularg hack here and we're
diff --git a/Src/subst.c b/Src/subst.c
index 5f993d6..a2bb648 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -1306,7 +1306,7 @@ get_intarg(char **s, int *delmatchp)
     p = dupstring(*s + arglen);
     *s = t + arglen;
     *t = sav;
-    if (parsestr(p))
+    if (parsestr(&p))
 	return -1;
     singsub(&p);
     if (errflag)
@@ -1329,7 +1329,8 @@ subst_parse_str(char **sp, int single, int err)
 
     *sp = s = dupstring(*sp);
 
-    if (!(err ? parsestr(s) : parsestrnoerr(s))) {
+    if (!(err ? parsestr(&s) : parsestrnoerr(&s))) {
+	*sp = s;
 	if (!single) {
             int qt = 0;
 
@@ -1439,7 +1440,8 @@ check_colon_subscript(char *str, char **endp)
     }
     sav = **endp;
     **endp = '\0';
-    if (parsestr(str = dupstring(str)))
+    str = dupstring(str);
+    if (parsestr(&str))
 	return NULL;
     singsub(&str);
     remnulargs(str);
diff --git a/Src/utils.c b/Src/utils.c
index f8d2394..4561b5e 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -1515,7 +1515,7 @@ checkmailpath(char **s)
 		    setunderscore(*s);
 
 		    u = dupstring(u);
-		    if (! parsestr(u)) {
+		    if (!parsestr(&u)) {
 			singsub(&u);
 			zputs(u, shout);
 			fputc('\n', shout);
diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index 94d15f2..cf639fa 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -1663,3 +1663,10 @@
 0:SHLVL appears sensible when about to exit shell
 >2
 >2
+
+# The following tests the return behaviour of parsestr/parsestrnoerr
+  alias param-test-alias='print $'\''\x45xpanded in substitution'\' 
+  param='$(param-test-alias)'
+  print ${(e)param}
+0:Alias expansion in command substitution in parameter evaluation
+>Expanded in substitution



Messages sorted by: Reverse Date, Date, Thread, Author