Subject: make: allow quoted lhs in conditionals
To: None <tech-toolchain@netbsd.org>
From: Simon Gerraty <sjg@juniper.net>
List: tech-toolchain
Date: 04/07/2004 18:17:42
The patch below, adds the ability to use quoted strings on the lhs of
conditionals.  This allows handling expressions which currently
generate errors due to a variable reference being expanded to nothing
before getting to Cond_Eval().

The unit-test adds a couple of cases to show the effect.

The basic approach is to extract all the string parsing in CondToken()
to a seperate function and call that for both lhs and rhs.

--sjg

Index: cond.c
===================================================================
RCS file: /cvsroot/src/usr.bin/make/cond.c,v
retrieving revision 1.20
diff -u -p -r1.20 cond.c
--- cond.c	8 Apr 2004 00:59:01 -0000	1.20
+++ cond.c	8 Apr 2004 01:16:22 -0000
@@ -526,6 +526,88 @@ CondCvtArg(char *str, double *value)
     }
 }
 
+static char *
+CondGetString(Boolean doEval, Boolean *quoted, Boolean *doFree)
+{
+    Buffer buf;
+    char *cp;
+    char *str;
+    int	len;
+    int qt;
+    char *start;
+
+    buf = Buf_Init(0);
+    str = NULL;
+    *quoted = qt = *condExpr == '"' ? 1 : 0;
+    if (qt)
+	condExpr++;
+    for (start = condExpr; *condExpr && str == NULL; condExpr++) {
+	switch (*condExpr) {
+	case '\\':
+	    if (condExpr[1] != '\0') {
+		condExpr++;
+		Buf_AddByte(buf, (Byte)*condExpr);
+	    }
+	    break;
+	case '"':
+	    if (qt) {
+		condExpr++;
+		goto got_str;
+	    } else
+		Buf_AddByte(buf, (Byte)*condExpr); /* likely? */
+	    break;
+	case ' ':
+	case '\t':
+	    if (!qt)
+		goto got_str;
+	    else
+		Buf_AddByte(buf, (Byte)*condExpr);
+	    break;
+	case '$':
+	    str = Var_Parse(condExpr, VAR_CMD, doEval, &len, doFree);
+	    if (str == var_Error) {
+		/*
+		 * Even if !doEval, we still report syntax errors, which
+		 * is what getting var_Error back with !doEval means.
+		 */
+		str = NULL;
+		goto cleanup;
+	    }
+	    condExpr += len;
+	    /*
+	     * If the '$' was first char, and we are followed by
+	     * space or the operator, we are done.
+	     */
+	    if ((condExpr == start + len) &&
+		(isspace((unsigned char) *condExpr) ||
+		 strchr("!=><", *condExpr))) {
+		goto cleanup;
+	    }
+	    /*
+	     * Nope, we better copy str to buf
+	     */
+	    for (cp = str; *cp; cp++) {
+		Buf_AddByte(buf, (Byte)*cp);
+	    }
+	    if (*doFree)
+		free(str);
+	    *doFree = FALSE;
+	    str = NULL;			/* not finished yet */
+	    break;
+	default:
+	    Buf_AddByte(buf, (Byte)*condExpr);
+	    break;
+	}
+    }
+ got_str:
+    Buf_AddByte(buf, (Byte)'\0');
+    str = (char *)Buf_GetAll(buf, NULL);
+    *doFree = TRUE;
+ cleanup:
+    Buf_Destroy(buf, FALSE);
+    return str;
+}
+
 /*-
  *-----------------------------------------------------------------------
  * CondToken --
@@ -580,52 +662,27 @@ CondToken(Boolean doEval)
 	    case '\0':
 		t = EndOfFile;
 		break;
+	    case '"':
 	    case '$': {
 		char	*lhs;
 		char	*rhs;
 		char	*op;
-		int	varSpecLen;
-		Boolean	doFree;
-
+		Boolean	lhsFree;
+		Boolean	rhsFree;
+		Boolean lhsQuoted;
+		Boolean rhsQuoted;
+
+		lhsFree = rhsFree = FALSE;
+		lhsQuoted = rhsQuoted = FALSE;
+		
 		/*
 		 * Parse the variable spec and skip over it, saving its
 		 * value in lhs.
 		 */
 		t = Err;
-		lhs = Var_Parse(condExpr, VAR_CMD, doEval,&varSpecLen,&doFree);
-		if (lhs == var_Error) {
-		    /*
-		     * Even if !doEval, we still report syntax errors, which
-		     * is what getting var_Error back with !doEval means.
-		     */
-		    return(Err);
-		}
-		condExpr += varSpecLen;
-
-		if (!isspace((unsigned char) *condExpr) &&
-		    strchr("!=><", *condExpr) == NULL) {
-		    Buffer buf;
-		    char *cp;
-
-		    buf = Buf_Init(0);
-
-		    for (cp = lhs; *cp; cp++)
-			Buf_AddByte(buf, (Byte)*cp);
-
-		    if (doFree)
-			free(lhs);
-
-		    for (;*condExpr && !isspace((unsigned char) *condExpr);
-			 condExpr++)
-			Buf_AddByte(buf, (Byte)*condExpr);
-
-		    Buf_AddByte(buf, (Byte)'\0');
-		    lhs = (char *)Buf_GetAll(buf, &varSpecLen);
-		    Buf_Destroy(buf, FALSE);
-
-		    doFree = TRUE;
-		}
-
+		lhs = CondGetString(doEval, &lhsQuoted, &lhsFree);
+		if (!lhs)
+		    return Err;
 		/*
 		 * Skip whitespace to get to the operator
 		 */
@@ -663,18 +720,11 @@ CondToken(Boolean doEval)
 				"Missing right-hand-side of operator");
 		    goto error;
 		}
-		rhs = condExpr;
+		rhs = CondGetString(doEval, &rhsQuoted, &rhsFree);
+		if (!rhs)
+		    return Err;
 do_compare:
-		if (*rhs == '"') {
-		    /*
-		     * Doing a string comparison. Only allow == and != for
-		     * operators.
-		     */
-		    char    *string;
-		    char    *cp, *cp2;
-		    int	    qt;
-		    Buffer  buf;
-
+		if (rhsQuoted || lhsQuoted) {
 do_string_compare:
 		    if (((*op != '!') && (*op != '=')) || (op[1] != '=')) {
 			Parse_Error(PARSE_WARNING,
@@ -682,63 +732,18 @@ do_string_compare:
 			goto error;
 		    }
 
-		    buf = Buf_Init(0);
-		    qt = *rhs == '"' ? 1 : 0;
-
-		    for (cp = &rhs[qt];
-			 ((qt && (*cp != '"')) ||
-			  (!qt && strchr(" \t)", *cp) == NULL)) &&
-			 (*cp != '\0'); cp++) {
-			if ((*cp == '\\') && (cp[1] != '\0')) {
-			    /*
-			     * Backslash escapes things -- skip over next
-			     * character, if it exists.
-			     */
-			    cp++;
-			    Buf_AddByte(buf, (Byte)*cp);
-			} else if (*cp == '$') {
-			    int	len;
-			    Boolean freeIt;
-
-			    cp2 = Var_Parse(cp, VAR_CMD, doEval,&len, &freeIt);
-			    if (cp2 != var_Error) {
-				Buf_AddBytes(buf, strlen(cp2), (Byte *)cp2);
-				if (freeIt) {
-				    free(cp2);
-				}
-				cp += len - 1;
-			    } else {
-				Buf_AddByte(buf, (Byte)*cp);
-			    }
-			} else {
-			    Buf_AddByte(buf, (Byte)*cp);
-			}
-		    }
-
-		    Buf_AddByte(buf, (Byte)0);
-
-		    string = (char *)Buf_GetAll(buf, (int *)0);
-		    Buf_Destroy(buf, FALSE);
-
 		    if (DEBUG(COND)) {
 			printf("lhs = \"%s\", rhs = \"%s\", op = %.2s\n",
-			       lhs, string, op);
+			       lhs, rhs, op);
 		    }
 		    /*
 		     * Null-terminate rhs and perform the comparison.
 		     * t is set to the result.
 		     */
 		    if (*op == '=') {
-			t = strcmp(lhs, string) ? False : True;
+			t = strcmp(lhs, rhs) ? False : True;
 		    } else {
-			t = strcmp(lhs, string) ? True : False;
-		    }
-		    free(string);
-		    if (rhs == condExpr) {
-		    	if (!qt && *cp == ')')
-			    condExpr = cp;
-			else
-			    condExpr = cp + 1;
+			t = strcmp(lhs, rhs) ? True : False;
 		    }
 		} else {
 		    /*
@@ -746,44 +751,13 @@ do_string_compare:
 		     * lhs and the rhs to a double and compare the two.
 		     */
 		    double  	left, right;
-		    char    	*string;
-
+		    char	*cp;
+		
 		    if (CondCvtArg(lhs, &left))
 			goto do_string_compare;
-		    if (*rhs == '$') {
-			int 	len;
-			Boolean	freeIt;
-
-			string = Var_Parse(rhs, VAR_CMD, doEval,&len,&freeIt);
-			if (string == var_Error) {
-			    right = 0.0;
-			} else {
-			    if (CondCvtArg(string, &right)) {
-				if (freeIt)
-				    free(string);
-				goto do_string_compare;
-			    }
-			    if (freeIt)
-				free(string);
-			    if (rhs == condExpr)
-				condExpr += len;
-			}
-		    } else {
-			char *cp;
-
-			if ((cp = CondCvtArg(rhs, &right)) &&
+		    if ((cp = CondCvtArg(rhs, &right)) &&
 			    cp == rhs)
-			    goto do_string_compare;
-			if (rhs == condExpr) {
-			    /*
-			     * Skip over the right-hand side
-			     */
-			    if (cp)
-				condExpr = cp;
-			    else
-				condExpr = strchr(rhs, '\0');	
-			}
-		    }
+			goto do_string_compare;
 
 		    if (DEBUG(COND)) {
 			printf("left = %f, right = %f, op = %.2s\n", left,
@@ -823,8 +797,10 @@ do_string_compare:
 		    }
 		}
 error:
-		if (doFree)
+		if (lhsFree)
 		    free(lhs);
+		if (rhsFree)
+		    free(rhs);
 		break;
 	    }
 	    default: {
Index: unit-tests/cond1
===================================================================
RCS file: /cvsroot/src/usr.bin/make/unit-tests/cond1,v
retrieving revision 1.1
diff -u -p -r1.1 cond1
--- unit-tests/cond1	8 Apr 2004 00:59:01 -0000	1.1
+++ unit-tests/cond1	8 Apr 2004 01:16:22 -0000
@@ -32,6 +32,13 @@ B=this should be an error
 B=unknown
 .endif
 
+.if "quoted" == quoted
+C=clever
+.else
+C=dim
+.endif
+
 all:
 	@echo "$n is $X prime"
-	@echo "A='$A' B='$B'"
+	@echo "A='$A' B='$B' C='$C'"
+	@echo "${NUMBERS:@n@$n is ${("${PRIMES:M$n}" == ""):?not:} prime${.newline}@}"
Index: unit-tests/test.exp
===================================================================
RCS file: /cvsroot/src/usr.bin/make/unit-tests/test.exp,v
retrieving revision 1.9
diff -u -p -r1.9 test.exp
--- unit-tests/test.exp	8 Apr 2004 00:59:01 -0000	1.9
+++ unit-tests/test.exp	8 Apr 2004 01:16:22 -0000
@@ -1,7 +1,13 @@
 make: "cond1" line 18: warning: extra else
 make: "cond1" line 28: warning: extra else
 2 is  prime
-A='other' B='unknown'
+A='other' B='unknown' C='clever'
+1 is not prime
+2 is  prime
+3 is  prime
+4 is not prime
+5 is  prime
+
 LIB=a X_LIBS:M${LIB${LIB:tu}} is "/tmp/liba.a"
 LIB=a X_LIBS:M*/lib${LIB}.a is "/tmp/liba.a"
 LIB=a X_LIBS:M*/lib${LIB}.a:tu is "/TMP/LIBA.A"