20412, tweaked: Make test builtin more POSIX compatible

20435: Make pattern.c not alter strings.

ChangeLog

@@ -1,3 +1,13 @@
+2004-10-05  Peter Stephenson  <pws@csr.com>
+
+	* 20435: Src/pattern.c: fix crash in negative matching.  Could do
+	with being streamlined but that might be a big change.
+
+	* 20412 (tweaked): Doc/Zsh/builtins.yo, Src/builtin.c, Src/cond.c,
+	Src/exec.c, Src/utils.c: Make test and [ builtins more POSIX
+	compatible: return status 2 for errors, require integer
+	constants for numeric tests.
+
 2004-10-01  Wayne Davison  <wayned@users.sourceforge.net>
 
 	* 20438: Src/builtin.c, Src/hist.c, Src/params.c, Src/zsh.h:

Doc/Zsh/builtins.yo

@@ -1124,6 +1124,12 @@ xitem(tt(test) [ var(arg) ... ])
 item(tt([) [ var(arg) ... ] tt(]))(
 Like the system version of tt(test).  Added for compatibility;
 use conditional expressions instead (see noderef(Conditional Expressions)).
+The main differences between the conditional expression syntax and the
+tt(test) and tt([) builtins are:  these commands are not handled
+syntactically, so for example an empty variable expansion may cause an
+argument to be omitted; syntax errors cause status 2 to be returned instead
+of a shell error; and arithmetic operators expect integer arguments rather
+than arithemetic expressions.
 )
 findex(times)
 cindex(shell, timing)

Src/builtin.c

@@ -4851,7 +4851,7 @@ bin_test(char *name, char **argv, UNUSED(Options ops), int func)
     state.strs = prog->strs;
 
 
-    return !evalcond(&state);
+    return evalcond(&state, name);
 }
 
 /* display a time, provided in units of 1/60s, as minutes and seconds */

Src/cond.c

@@ -37,15 +37,26 @@ static char *condstr[COND_MOD] = {
     "-ne", "-lt", "-gt", "-le", "-ge"
 };
 
+/*
+ * Evaluate a conditional expression given the arguments.
+ * If fromtest is set, the caller is the test or [ builtin;
+ * with the pointer giving the name of the command.
+ * for POSIX conformance this supports a more limited range
+ * of functionality.
+ *
+ * Return status is the final shell status, i.e. 0 for true,
+ * 1 for false and 2 for error.
+ */
+
 /**/
 int
-evalcond(Estate state)
+evalcond(Estate state, char *fromtest)
 {
     struct stat *st;
     char *left, *right;
     Wordcode pcode;
     wordcode code;
-    int ctype, htok = 0;
+    int ctype, htok = 0, ret;
 
  rec:
 
@@ -58,24 +69,28 @@ evalcond(Estate state)
     case COND_NOT:
 	if (tracingcond)
 	    fprintf(xtrerr, " %s", condstr[ctype]);
-	return !evalcond(state);
+	ret = evalcond(state, fromtest);
+	if (ret == 2)
+	    return ret;
+	else
+	    return !ret;
     case COND_AND:
-	if (evalcond(state)) {
+	if (!(ret = evalcond(state, fromtest))) {
 	    if (tracingcond)
 		fprintf(xtrerr, " %s", condstr[ctype]);
 	    goto rec;
 	} else {
 	    state->pc = pcode + (WC_COND_SKIP(code) + 1);
-	    return 0;
+	    return ret;
 	}
     case COND_OR:
-	if (!evalcond(state)) {
+	if ((ret = evalcond(state, fromtest)) == 1) {
 	    if (tracingcond)
 		fprintf(xtrerr, " %s", condstr[ctype]);
 	    goto rec;
 	} else {
 	    state->pc = pcode + (WC_COND_SKIP(code) + 1);
-	    return 1;
+	    return ret;
 	}
     case COND_MOD:
     case COND_MODI:
@@ -99,12 +114,13 @@ evalcond(Estate state)
 	    if ((cd = getconddef((ctype == COND_MODI), name + 1, 1))) {
 		if (ctype == COND_MOD &&
 		    (l < cd->min || (cd->max >= 0 && l > cd->max))) {
-		    zerr("unrecognized condition: `%s'", name, 0);
+		    zwarnnam(fromtest, "unrecognized condition: `%s'",
-		    return 0;
+			     name, 0);
+		    return 2;
 		}
 		if (tracingcond)
 		    tracemodcond(name, strs, ctype == COND_MODI);
-		return cd->handler(strs, cd->condid);
+		return !cd->handler(strs, cd->condid);
 	    }
 	    else {
 		char *s = strs[0];
@@ -115,16 +131,20 @@ evalcond(Estate state)
 		if (name && name[0] == '-' &&
 		    (cd = getconddef(0, name + 1, 1))) {
 		    if (l < cd->min || (cd->max >= 0 && l > cd->max)) {
-			zerr("unrecognized condition: `%s'", name, 0);
+			zwarnnam(fromtest, "unrecognized condition: `%s'",
-			return 0;
+				 name, 0);
+			return 2;
 		    }
 		    if (tracingcond)
 			tracemodcond(name, strs, ctype == COND_MODI);
-		    return cd->handler(strs, cd->condid);
+		    return !cd->handler(strs, cd->condid);
-		} else
+		} else {
-		    zerr("unrecognized condition: `%s'", name, 0);
+		    zwarnnam(fromtest,
+			     "unrecognized condition: `%s'", name, 0);
 		}
-	    return 0;
+	    }
+	    /* module not found, error */
+	    return 2;
 	}
     }
     left = ecgetstr(state, EC_DUPTOK, &htok);
@@ -159,8 +179,34 @@ evalcond(Estate state)
 
     if (ctype >= COND_EQ && ctype <= COND_GE) {
 	mnumber mn1, mn2;
+	if (fromtest) {
+	    /*
+	     * For test and [, the expressions must be base 10 integers,
+	     * not integer expressions.
+	     */
+	    char *eptr, *err;
+
+	    mn1.u.l = zstrtol(left, &eptr, 10);
+	    if (!*eptr)
+	    {
+		mn2.u.l = zstrtol(right, &eptr, 10);
+		err = right;
+	    }
+	    else
+		err = left;
+
+	    if (*eptr)
+	    {
+		zwarnnam(fromtest, "integer expression expected: %s",
+			 err, 0);
+		return 2;
+	    }
+
+	    mn1.type = mn2.type = MN_INTEGER;
+	} else {
 	    mn1 = matheval(left);
 	    mn2 = matheval(right);
+	}
 
 	if (((mn1.type|mn2.type) & (MN_INTEGER|MN_FLOAT)) ==
 	    (MN_INTEGER|MN_FLOAT)) {
@@ -176,23 +222,23 @@ evalcond(Estate state)
 	}
 	switch(ctype) {
 	case COND_EQ:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d == mn2.u.d) :
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d == mn2.u.d) :
-		(mn1.u.l == mn2.u.l);
+		     (mn1.u.l == mn2.u.l));
 	case COND_NE:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d != mn2.u.d) :
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d != mn2.u.d) :
-		(mn1.u.l != mn2.u.l);
+		     (mn1.u.l != mn2.u.l));
 	case COND_LT:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d < mn2.u.d) :
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d < mn2.u.d) :
-		(mn1.u.l < mn2.u.l);
+		     (mn1.u.l < mn2.u.l));
 	case COND_GT:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d > mn2.u.d) :
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d > mn2.u.d) :
-		(mn1.u.l > mn2.u.l);
+		     (mn1.u.l > mn2.u.l));
 	case COND_LE:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d <= mn2.u.d) :
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d <= mn2.u.d) :
-		(mn1.u.l <= mn2.u.l);
+		     (mn1.u.l <= mn2.u.l));
 	case COND_GE:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d >= mn2.u.d) :
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d >= mn2.u.d) :
-		(mn1.u.l >= mn2.u.l);
+		     (mn1.u.l >= mn2.u.l));
 	}
     }
 
@@ -215,81 +261,83 @@ evalcond(Estate state)
 			!strcmp(opat, right) && pprog != dummy_patprog2);
 
 		if (!(pprog = patcompile(right, (save ? PAT_ZDUP : PAT_STATIC),
-					 NULL)))
+					 NULL))) {
-		    zerr("bad pattern: %s", right, 0);
+		    zwarnnam(fromtest, "bad pattern: %s", right, 0);
+		    return 2;
+		}
 		else if (save)
 		    state->prog->pats[npat] = pprog;
 	    }
 	    state->pc += 2;
 	    test = (pprog && pattry(pprog, left));
 
-	    return (ctype == COND_STREQ ? test : !test);
+	    return !(ctype == COND_STREQ ? test : !test);
 	}
     case COND_STRLT:
-	return strcmp(left, right) < 0;
+	return !(strcmp(left, right) < 0);
     case COND_STRGTR:
-	return strcmp(left, right) > 0;
+	return !(strcmp(left, right) > 0);
     case 'e':
     case 'a':
-	return (doaccess(left, F_OK));
+	return (!doaccess(left, F_OK));
     case 'b':
-	return (S_ISBLK(dostat(left)));
+	return (!S_ISBLK(dostat(left)));
     case 'c':
-	return (S_ISCHR(dostat(left)));
+	return (!S_ISCHR(dostat(left)));
     case 'd':
-	return (S_ISDIR(dostat(left)));
+	return (!S_ISDIR(dostat(left)));
     case 'f':
-	return (S_ISREG(dostat(left)));
+	return (!S_ISREG(dostat(left)));
     case 'g':
-	return (!!(dostat(left) & S_ISGID));
+	return (!(dostat(left) & S_ISGID));
     case 'k':
-	return (!!(dostat(left) & S_ISVTX));
+	return (!(dostat(left) & S_ISVTX));
     case 'n':
-	return (!!strlen(left));
+	return (!strlen(left));
     case 'o':
-	return (optison(left));
+	return (optison(fromtest, left));
     case 'p':
-	return (S_ISFIFO(dostat(left)));
+	return (!S_ISFIFO(dostat(left)));
     case 'r':
-	return (doaccess(left, R_OK));
+	return (!doaccess(left, R_OK));
     case 's':
-	return ((st = getstat(left)) && !!(st->st_size));
+	return !((st = getstat(left)) && !!(st->st_size));
     case 'S':
-	return (S_ISSOCK(dostat(left)));
+	return (!S_ISSOCK(dostat(left)));
     case 'u':
-	return (!!(dostat(left) & S_ISUID));
+	return (!(dostat(left) & S_ISUID));
     case 'w':
-	return (doaccess(left, W_OK));
+	return (!doaccess(left, W_OK));
     case 'x':
 	if (privasserted()) {
 	    mode_t mode = dostat(left);
-	    return (mode & S_IXUGO) || S_ISDIR(mode);
+	    return !((mode & S_IXUGO) || S_ISDIR(mode));
 	}
-	return doaccess(left, X_OK);
+	return !doaccess(left, X_OK);
     case 'z':
-	return (!strlen(left));
+	return !!(strlen(left));
     case 'h':
     case 'L':
-	return (S_ISLNK(dolstat(left)));
+	return (!S_ISLNK(dolstat(left)));
     case 'O':
-	return ((st = getstat(left)) && st->st_uid == geteuid());
+	return !((st = getstat(left)) && st->st_uid == geteuid());
     case 'G':
-	return ((st = getstat(left)) && st->st_gid == getegid());
+	return !((st = getstat(left)) && st->st_gid == getegid());
     case 'N':
-	return ((st = getstat(left)) && st->st_atime <= st->st_mtime);
+	return !((st = getstat(left)) && st->st_atime <= st->st_mtime);
     case 't':
-	return isatty(mathevali(left));
+	return !isatty(mathevali(left));
     case COND_NT:
     case COND_OT:
 	{
 	    time_t a;
 
 	    if (!(st = getstat(left)))
-		return 0;
+		return 1;
 	    a = st->st_mtime;
 	    if (!(st = getstat(right)))
-		return 0;
+		return 2;
-	    return (ctype == COND_NT) ? a > st->st_mtime : a < st->st_mtime;
+	    return !((ctype == COND_NT) ? a > st->st_mtime : a < st->st_mtime);
 	}
     case COND_EF:
 	{
@@ -297,17 +345,18 @@ evalcond(Estate state)
 	    ino_t i;
 
 	    if (!(st = getstat(left)))
-		return 0;
+		return 1;
 	    d = st->st_dev;
 	    i = st->st_ino;
 	    if (!(st = getstat(right)))
-		return 0;
+		return 1;
-	    return d == st->st_dev && i == st->st_ino;
+	    return !(d == st->st_dev && i == st->st_ino);
 	}
     default:
-	zerr("bad cond code", NULL, 0);
+	zwarnnam(fromtest, "bad cond code", NULL, 0);
+	return 2;
     }
-    return 0;
+    return 1;
 }
 
 
@@ -371,9 +420,13 @@ dolstat(char *s)
 }
 
 
+/*
+ * optison returns evalcond-friendly statuses (true, false, error).
+ */
+
 /**/
 static int
-optison(char *s)
+optison(char *name, char *s)
 {
     int i;
 
@@ -382,12 +435,12 @@ optison(char *s)
     else
 	i = optlookup(s);
     if (!i) {
-	zerr("no such option: %s", s, 0);
+	zwarnnam(name, "no such option: %s", s, 0);
-	return 0;
+	return 2;
     } else if(i < 0)
-	return unset(-i);
+	return !unset(-i);
     else
-	return isset(i);
+	return !isset(i);
 }
 
 /**/

Src/exec.c

@@ -3187,7 +3187,13 @@ execcond(Estate state, UNUSED(int do_exec))
 	tracingcond++;
     }
     cmdpush(CS_COND);
-    stat = !evalcond(state);
+    stat = evalcond(state, NULL);
+    /*
+     * 2 indicates a syntax error.  For compatibility, turn this
+     * into a shell error.
+     */
+    if (stat == 2)
+	errflag = 1;
     cmdpop();
     if (isset(XTRACE)) {
 	fprintf(xtrerr, " ]]\n");

Src/pattern.c

@@ -1818,10 +1818,9 @@ patmatch(Upat prog)
 			syncstrp->p = (unsigned char *)zshcalloc(patinlen);
 			while ((ret = patmatch(P_OPERAND(scan)))) {
 			    unsigned char *syncpt;
-			    char savchar, *testptr;
+			    char *savpatinstart, *origsave, *origpatinstart;
-			    char *savpatinstart = patinstart;
 			    int savforce = forceerrs, savpatinlen = patinlen;
-			    int savpatflags = patflags;
+			    int savpatflags = patflags, synclen;
 			    forceerrs = -1;
 			    savglobdots = globdots;
 			    matchederrs = errsfound;
@@ -1837,16 +1836,38 @@ patmatch(Upat prog)
 			     */
 			    for (syncpt = syncstrp->p; !*syncpt; syncpt++)
 				;
-			    testptr = patinstart + (syncpt - syncstrp->p);
+			    synclen = syncpt - syncstrp->p;
-			    DPUTS(testptr > matchpt, "BUG: EXCSYNC failed");
+			    if (patinstart[synclen]) {
-			    savchar = *testptr;
+				/*
+				 * We need to truncate the string at
+				 * this point.  Copy a whole load of
+				 * stuff to avoid modifying the string.
+				 * This includes (at least) patinstart,
+				 * patinput and save.
+				 */
+				origsave = save;
+				origpatinstart = patinstart;
+
+				DPUTS(patinstart + synclen > matchpt,
+				      "BUG: EXCSYNC failed");
+
+				savpatinstart = patinstart =
+				    ztrduppfx(patinstart, synclen);
+				patinput = patinstart +
+				    (patinput - origpatinstart);
+				save = patinstart + (save - origpatinstart);
 				/*
 				 * If this isn't really the end of the string,
 				 * remember this for the (#e) assertion.
 				 */
-			    if (savchar)
 				patflags |= PAT_NOTEND;
-			    *testptr = '\0';
+			    }
+			    else
+			    {
+				/* Don't need to copy, already right length */
+				origsave = origpatinstart = NULL;
+				savpatinstart = patinstart;
+			    }
 			    next = PATNEXT(scan);
 			    while (next && P_ISEXCLUDE(next)) {
 				char *buf = NULL;
@@ -1893,7 +1914,17 @@ patmatch(Upat prog)
 				    break;
 				next = PATNEXT(next);
 			    }
-			    *testptr = savchar;
+			    /*
+			     * Free copied string and restore if
+			     * we needed to truncate.
+			     */
+			    if (origpatinstart) {
+				patinput = origpatinstart +
+				    (patinput - patinstart);
+				zfree(patinstart, synclen+1);
+				patinstart = origpatinstart;
+				save = origsave;
+			    }
 			    patflags = savpatflags;
 			    globdots = savglobdots;
 			    forceerrs = savforce;

Src/utils.c

@@ -96,6 +96,10 @@ zwarn(const char *fmt, const char *str, int num)
 mod_export void
 zwarnnam(const char *cmd, const char *fmt, const char *str, int num)
 {
+    if (!cmd) {
+	zwarn(fmt, str, num);
+	return;
+    }
     if (errflag || noerrs)
 	return;
     trashzle();
@@ -103,10 +107,8 @@ zwarnnam(const char *cmd, const char *fmt, const char *str, int num)
 	nicezputs(scriptname ? scriptname : argzero, stderr);
 	fputc((unsigned char)':', stderr);
     }
-    if (cmd) {
     nicezputs(cmd, stderr);
     fputc((unsigned char)':', stderr);
-    }
     zerrmsg(fmt, str, num);
 }