Re: More POSIX developments

[Peter Stephenson <pws@xxxxxxx>]

Bart Schaefer wrote:
> According to today's minutes of the yesterday's austin-group teleconf:
> 
> -----
> It was agreed so far that
> 
> test asdf -ge 0 
> 
> is a syntax error and is expected to return  something greater than 1
> -----
> 
> In zsh's builtin test, of course, "asdf -ge 0" is interpreted in math 
> context, and is equivalent to (( asdf >= 0 )) and thus to "$asdf -ge 0".  
> The (( )) form is still considered OK, but the test -ge form is (or soon 
> will be) required to fail.
> 
> Yet another case where "emulate posix" might be useful.  An idea that 
> occurs to me is that we could introduce hidden options, that is, having
> no name that could be referenced via "setopt" but that are switched on
> and off in groups, only via "emulate".

Well, [[ ... ]] has always been the preferred zsh way of doing things,
with [ and test for compatibility, so it's at least plausible that the
latter should simply implement POSIX.  The documentation clearly says
test was "added for compatibility", as well as vaguely indicating it's
"like" conditional expressions.  So there is some wiggle room this time.

Returning "something greater than 1" for an error is nastier to code,
since the main function involved, evalcond(), returns statuses the other
way round.  Hence a lot of return statuses need flipping.

The following patch, which I won't commit until we've decided which way
to go, tries to cover the bases by making test work (more) like POSIX
while leaving [[ ... ]] the way it is.  The errors now return status 2
from evalcond, but for backward compatibility [[ ... ]] turns them into
shell errors.

Quite possibly I've missed something, but this is about the minimal
damage we can inflict without an option.

The existing tests still all pass, although the new code means new tests
should be added.  Also, the documentation needs clarifying whatever we
decide to do.

I needed to add an extra warning function to choose between the cases
where we have a builtin name and where the code is called internally as
a condition.  Does anyone know if there's a reason zwarnnam handles the
absence of a command name in a strange way?  In other words, could I
remove zwarnnamopt and make zwarnnam behave like that?  The current
behaviour of zwarnnam without a command name doesn't look very useful:
it outputs a space and then the error message.  However, I haven't
checked all the zillion calls to it and maybe it's designed to be output
after another message.

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.126
diff -u -r1.126 builtin.c
--- Src/builtin.c	9 Sep 2004 10:12:47 -0000	1.126
+++ Src/builtin.c	27 Sep 2004 10:56:12 -0000
@@ -4846,7 +4846,7 @@
     state.strs = prog->strs;
 
 
-    return !evalcond(&state);
+    return evalcond(&state, name);
 }
 
 /* display a time, provided in units of 1/60s, as minutes and seconds */
Index: Src/cond.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/cond.c,v
retrieving revision 1.5
diff -u -r1.5 cond.c
--- Src/cond.c	1 Aug 2002 15:06:26 -0000	1.5
+++ Src/cond.c	27 Sep 2004 10:56:12 -0000
@@ -37,15 +37,26 @@
     "-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 @@
     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 @@
 	    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);
-		    return 0;
+		    zwarnnamopt(fromtest, "unrecognized condition: `%s'",
+				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 @@
 		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);
-			return 0;
+			zwarnnamopt(fromtest, "unrecognized condition: `%s'",
+				    name, 0);
+			return 2;
 		    }
 		    if (tracingcond)
 			tracemodcond(name, strs, ctype == COND_MODI);
-		    return cd->handler(strs, cd->condid);
-		} else
-		    zerr("unrecognized condition: `%s'", name, 0);
+		    return !cd->handler(strs, cd->condid);
+		} else {
+		    zwarnnamopt(fromtest,
+				"unrecognized condition: `%s'", name, 0);
+		}
 	    }
-	    return 0;
+	    /* module not found, error */
+	    return 2;
 	}
     }
     left = ecgetstr(state, EC_DUPTOK, &htok);
@@ -159,8 +179,34 @@
 
     if (ctype >= COND_EQ && ctype <= COND_GE) {
 	mnumber mn1, mn2;
-	mn1 = matheval(left);
-	mn2 = matheval(right);
+	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)
+	    {
+		zwarnnamopt(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 @@
 	}
 	switch(ctype) {
 	case COND_EQ:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d == mn2.u.d) :
-		(mn1.u.l == mn2.u.l);
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d == mn2.u.d) :
+		     (mn1.u.l == mn2.u.l));
 	case COND_NE:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d != mn2.u.d) :
-		(mn1.u.l != mn2.u.l);
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d != mn2.u.d) :
+		     (mn1.u.l != mn2.u.l));
 	case COND_LT:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d < mn2.u.d) :
-		(mn1.u.l < mn2.u.l);
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d < mn2.u.d) :
+		     (mn1.u.l < mn2.u.l));
 	case COND_GT:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d > mn2.u.d) :
-		(mn1.u.l > mn2.u.l);
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d > mn2.u.d) :
+		     (mn1.u.l > mn2.u.l));
 	case COND_LE:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d <= mn2.u.d) :
-		(mn1.u.l <= mn2.u.l);
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d <= mn2.u.d) :
+		     (mn1.u.l <= mn2.u.l));
 	case COND_GE:
-	    return (mn1.type & MN_FLOAT) ? (mn1.u.d >= mn2.u.d) :
-		(mn1.u.l >= mn2.u.l);
+	    return !((mn1.type & MN_FLOAT) ? (mn1.u.d >= mn2.u.d) :
+		     (mn1.u.l >= mn2.u.l));
 	}
     }
 
@@ -215,81 +261,83 @@
 			!strcmp(opat, right) && pprog != dummy_patprog2);
 
 		if (!(pprog = patcompile(right, (save ? PAT_ZDUP : PAT_STATIC),
-					 NULL)))
-		    zerr("bad pattern: %s", right, 0);
+					 NULL))) {
+		    zwarnnamopt(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 (ctype == COND_NT) ? a > st->st_mtime : a < st->st_mtime;
+		return 2;
+	    return !((ctype == COND_NT) ? a > st->st_mtime : a < st->st_mtime);
 	}
     case COND_EF:
 	{
@@ -297,17 +345,18 @@
 	    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 d == st->st_dev && i == st->st_ino;
+		return 1;
+	    return !(d == st->st_dev && i == st->st_ino);
 	}
     default:
-	zerr("bad cond code", NULL, 0);
+	zwarnnamopt(fromtest, "bad cond code", NULL, 0);
+	return 2;
     }
-    return 0;
+    return 1;
 }
 
 
@@ -371,9 +420,13 @@
 }
 
 
+/*
+ * optison returns evalcond-friendly statuses (true, false, error).
+ */
+
 /**/
 static int
-optison(char *s)
+optison(char *name, char *s)
 {
     int i;
 
@@ -382,12 +435,12 @@
     else
 	i = optlookup(s);
     if (!i) {
-	zerr("no such option: %s", s, 0);
-	return 0;
+	zwarnnamopt(name, "no such option: %s", s, 0);
+	return 2;
     } else if(i < 0)
-	return unset(-i);
+	return !unset(-i);
     else
-	return isset(i);
+	return !isset(i);
 }
 
 /**/
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.71
diff -u -r1.71 exec.c
--- Src/exec.c	8 Sep 2004 08:24:41 -0000	1.71
+++ Src/exec.c	27 Sep 2004 10:56:13 -0000
@@ -3187,7 +3187,13 @@
 	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");
Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.65
diff -u -r1.65 utils.c
--- Src/utils.c	17 Sep 2004 09:25:42 -0000	1.65
+++ Src/utils.c	27 Sep 2004 10:56:15 -0000
@@ -110,6 +110,22 @@
     zerrmsg(fmt, str, num);
 }
 
+/*
+ * zwarnnamopt is used in cases for code which is called both
+ * from a builtin (cmd non-NULL) and internally (cmd NULL).
+ * Is there a good reason zwarnnam doesn't do this?
+ */
+
+/**/
+mod_export void
+zwarnnamopt(const char *cmd, const char *fmt, const char *str, int num)
+{
+    if (cmd)
+	zwarnnam(cmd, fmt, str, num);
+    else
+	zwarn(fmt, str, num);
+}
+
 #ifdef __CYGWIN__
 /*
  * This works around an occasional problem with dllwrap on Cygwin, seen

-- 
Peter Stephenson <pws@xxxxxxx>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************