Patch to fix test(1) expression parsing

[Alex Musolino <alex@musolino.id.au>]

Hi all,

The following patch fixes test(1) to correctly parse its own
expression syntax whilst avoiding executing of filesystem operations
when these tests ought not to be performed due to short-circuiting.

Previously, short-circuiting of -a and -o logical operators would
result in an incorrect parse.  For example, the following invocations
fail:

term% test '(' 1 -gt 2 -a 1 -gt 1 ')'
test: ) expected
term% test '(' 2 -gt 1 -o 1 -gt 1 ')'
test: ) expected

Comments/feedback/testing welcome.

diff -r 7510f0e7ba7f sys/src/cmd/test.c
--- a/sys/src/cmd/test.c	Thu Jul 30 15:59:04 2020 +0200
+++ b/sys/src/cmd/test.c	Sun Aug 02 01:19:39 2020 +0930
@@ -30,7 +30,7 @@
 int	isnewerthan(char *, char *);
 int	hasmode(char *, ulong);
 int	tio(char *, int);
-int	e(void), e1(void), e2(void), e3(void);
+int	e(int), e0(int), e1(int), e2(int), e3(int);
 char	*nxtarg(int);
 
 void
@@ -47,7 +47,7 @@
 	argv[ac] = 0;
 	if (ac<=1)
 		exits("usage");
-	r = e();
+	r = e(1);
 	/*
 	 * nice idea but short-circuit -o and -a operators may have
 	 * not consumed their right-hand sides.
@@ -81,78 +81,116 @@
 }
 
 int
-e(void)
+e(int eval)
 {
+	char *a;
 	int p1;
 
-	p1 = e1();
-	if (EQ(nxtarg(1), "-o"))
-		return(p1 || e());
+	p1 = e0(eval);
+	a = nxtarg(0);
+	if (EQ(a, "-o")){
+		if(p1){
+			e0(0);
+			return 1;
+		}
+		return e0(eval);
+	}
 	ap--;
 	return(p1);
 }
 
 int
-e1(void)
+e0(int eval)
 {
 	int p1;
 
-	p1 = e2();
-	if (EQ(nxtarg(1), "-a"))
-		return (p1 && e1());
+	if(EQ(nxtarg(0), "(")){
+		p1 = e1(eval);
+		if(!EQ(nxtarg(0), ")"))
+			synbad(") expected","");
+		return p1;
+	}
+	ap--;
+	return e1(eval);
+}
+
+int
+e1(int eval)
+{
+	int p1;
+
+	p1 = e2(eval);
+	if (EQ(nxtarg(0), "-a")){
+		if(p1)
+			return e(eval);
+		e(0);
+		return 0;
+	}
 	ap--;
 	return(p1);
 }
 
 int
-e2(void)
+e2(int eval)
 {
 	if (EQ(nxtarg(0), "!"))
-		return(!e2());
+		return(!e(eval));
 	ap--;
-	return(e3());
+	return(e3(eval));
 }
 
 int
-e3(void)
+e3(int eval)
 {
-	int p1, int1, int2;
-	char *a, *p2;
+	int int1, int2;
+	char *a, *b, *p2;
 
 	a = nxtarg(0);
-	if(EQ(a, "(")) {
-		p1 = e();
-		if(!EQ(nxtarg(0), ")"))
-			synbad(") expected","");
-		return(p1);
+
+	if(EQ(a, "-A")){
+		b = nxtarg(0);
+		return(eval && hasmode(b, DMAPPEND));
 	}
 
-	if(EQ(a, "-A"))
-		return(hasmode(nxtarg(0), DMAPPEND));
+	if(EQ(a, "-L")){
+		b = nxtarg(0);
+		return(eval && hasmode(b, DMEXCL));
+	}
 
-	if(EQ(a, "-L"))
-		return(hasmode(nxtarg(0), DMEXCL));
+	if(EQ(a, "-T")){
+		b = nxtarg(0);
+		return(eval && hasmode(b, DMTMP));
+	}
 
-	if(EQ(a, "-T"))
-		return(hasmode(nxtarg(0), DMTMP));
+	if(EQ(a, "-f")){
+		b = nxtarg(0);
+		return(eval && isreg(b));
+	}
 
-	if(EQ(a, "-f"))
-		return(isreg(nxtarg(0)));
+	if(EQ(a, "-d")){
+		b = nxtarg(0);
+		return(eval && isdir(b));
+	}
 
-	if(EQ(a, "-d"))
-		return(isdir(nxtarg(0)));
+	if(EQ(a, "-r")){
+		b = nxtarg(0);
+		return(eval && tio(b, AREAD));
+	}
 
-	if(EQ(a, "-r"))
-		return(tio(nxtarg(0), AREAD));
+	if(EQ(a, "-w")){
+		b = nxtarg(0);
+		return(eval && tio(b, AWRITE));
+	}
 
-	if(EQ(a, "-w"))
-		return(tio(nxtarg(0), AWRITE));
+	if(EQ(a, "-x")){
+		b = nxtarg(0);
+		return(eval && tio(b, AEXEC));
+	}
 
-	if(EQ(a, "-x"))
-		return(tio(nxtarg(0), AEXEC));
-
-	if(EQ(a, "-e"))
-		return(tio(nxtarg(0), AEXIST));
+	if(EQ(a, "-e")){
+		b = nxtarg(0);
+		return(eval && tio(b, AEXIST));
+	}
 
 	if(EQ(a, "-c"))
 		return(0);
@@ -166,14 +204,16 @@
 	if(EQ(a, "-g"))
 		return(0);
 
-	if(EQ(a, "-s"))
-		return(fsizep(nxtarg(0)));
+	if(EQ(a, "-s")){
+		b = nxtarg(0);
+		return(eval && fsizep(b));
+	}
 
 	if(EQ(a, "-t"))
 		if(ap>=ac)
-			return(isatty(1));
+			return(eval && isatty(1));
 		else if(nxtintarg(&int1))
-			return(isatty(int1));
+			return(eval && isatty(int1));
 		else
 			synbad("not a valid file descriptor number ", "");
 
@@ -182,7 +222,8 @@
 	if(EQ(a, "-z"))
 		return(EQ(nxtarg(0), ""));
 
-	p2 = nxtarg(1);
+	p2 = nxtarg(0);
+
 	if (p2==0)
 		return(!EQ(a,""));
 	if(EQ(p2, "="))
@@ -191,14 +232,20 @@
 	if(EQ(p2, "!="))
 		return(!EQ(nxtarg(0), a));
 
-	if(EQ(p2, "-older"))
-		return(isolder(nxtarg(0), a));
+	if(EQ(p2, "-older")){
+		b = nxtarg(0);
+		return(eval && isolder(b, a));
+	}
 
-	if(EQ(p2, "-ot"))
-		return(isolderthan(nxtarg(0), a));
+	if(EQ(p2, "-ot")){
+		b = nxtarg(0);
+		return(eval && isolderthan(b, a));
+	}
 
-	if(EQ(p2, "-nt"))
-		return(isnewerthan(nxtarg(0), a));
+	if(EQ(p2, "-nt")){
+		b = nxtarg(0);
+		return(eval && isnewerthan(b, a));
+	}
 
 	if(!isint(a, &int1))
 		synbad("unexpected operator/operand: ", p2);