More POSIX developments

More POSIX developments

[Bart Schaefer]

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".

Re: More POSIX developments

[Peter Stephenson]

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@csr.com>                  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
**********************************************************************

Re: More POSIX developments

[Bart Schaefer]

Finally got a chance to look at this.

On Mon, 27 Sep 2004, Peter Stephenson wrote:

> 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.

I think this would be OK.

> 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?

I paged through a grep of all the calls to zwarnnam(), and there don't
seem to be any that intentionally get passed a NULL first argument, so it
probably is just defensive programming.

> In other words, could I remove zwarnnamopt and make zwarnnam behave like 
> that?

I think that'd be safe, but I wouldn't object to a second opinion.

Re: More POSIX developments

[Peter Stephenson]

Bart Schaefer wrote:
> Finally got a chance to look at this.
> 
> On Mon, 27 Sep 2004, Peter Stephenson wrote:
> 
> > 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.
> 
> I think this would be OK.

Yes, the more I think about it the less likely it is that someone using
"test" or "[" syntax wants anything other than standard-compliant
behaviour.

> > In other words, could I remove zwarnnamopt and make zwarnnam behave like 
> > that?
> 
> I think that'd be safe, but I wouldn't object to a second opinion.

I made the change and none of the tests failed.  I think it's neater to
do it like this and see if any problems turn up.

Before I commit the patch, here's some proposed documentation.

Index: Doc/Zsh/builtins.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v
retrieving revision 1.70
diff -u -r1.70 builtins.yo
--- Doc/Zsh/builtins.yo	30 Jun 2004 11:10:45 -0000	1.70
+++ Doc/Zsh/builtins.yo	4 Oct 2004 10:21:15 -0000
@@ -1124,6 +1124,11 @@
 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 are:  this command is 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)

-- 
Peter Stephenson <pws@csr.com>                  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
**********************************************************************

Re: More POSIX developments

[Oliver Kiddle]

Peter wrote:

> Before I commit the patch, here's some proposed documentation.
> 
>  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 are:  this command is 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.

Reading that, it isn't clear whether the differences listed are relative
to "the system version of test" or to Conditional Expressions.

Oliver

Re: More POSIX developments

[Bart Schaefer]

On Mon, 4 Oct 2004, Peter Stephenson wrote:

> Yes, the more I think about it the less likely it is that someone using 
> "test" or "[" syntax wants anything other than standard-compliant 
> behaviour.

Just to keep things entertaining, this discusson was just revived on the 
austin-group list, and Geoff Clare is arguing in favor of permitting the
extension ...