Subject: Re: make: allow setting variable mods in variable?
To: Roland Illig <rillig@NetBSD.org>
From: Simon J. Gerraty <sjg@crufty.net>
List: tech-toolchain
Date: 02/12/2006 17:03:10
On Sun, 12 Feb 2006 08:28:19 +0100, Roland Illig writes:
>It's a great step towards usability and readability to write 
>${PREFIX:${Qsed}:Q} instead of ${PREFIX:C/\\/\\\\/g:C/&/\\&/g:Q}. As 
>well as Qawk, Qc, Qperl, Qpython, Qruby, etc.

Yes.  It keeps the implementation simple if we require the 
initial ':' to be explicit (ie not from the expansion of the modifiers).

>>>--- var.c.~1~	Sat Sep  3 15:15:09 2005
>>>+++ var.c	Fri Jan 20 16:58:52 2006
>>>@@ -1890,6 +1890,7 @@ Var_Parse(const char *str, GNode *ctxt, 
>>>	  Boolean *freePtr)
>>>{
>>>    const char	   *tstr;    	/* Pointer into str */
>>>+    const char	   *tstr2;	/* Secondary tstr if we need to recurse
>  */
>
>...
>
>>>+    if (tstr2)
>>>+	free(tstr2);
>
>Freeing a pointer to constant memory doesn't look nice to me.

True, I've made it non-const.  Also we need to free it in a couple of
other placess too - and replaced a return with goto cleanup.

This patch is against current, and passes unit-tests etc on 2.0

--sjg

Index: var.c
===================================================================
RCS file: /cvsroot/src/usr.bin/make/var.c,v
retrieving revision 1.100
diff -u -p -r1.100 var.c
--- var.c	27 Aug 2005 08:04:26 -0000	1.100
+++ var.c	13 Feb 2006 00:59:41 -0000
@@ -1890,6 +1890,8 @@ Var_Parse(const char *str, GNode *ctxt, 
 	  Boolean *freePtr)
 {
     const char	   *tstr;    	/* Pointer into str */
+    char	   *tstr2;	/* Secondary tstr if we need
+				 * to expand modifiers  */
     Var	    	   *v;	    	/* Variable in invocation */
     const char     *cp;    	/* Secondary pointer into str (place marker
 				 * for tstr) */
@@ -1915,6 +1917,7 @@ Var_Parse(const char *str, GNode *ctxt, 
     start = str;
     parsestate.oneBigWord = FALSE;
     parsestate.varSpace = ' ';	/* word separator */
+    tstr2 = NULL;
 
     if (str[1] != PROPEN && str[1] != BROPEN) {
 	/*
@@ -2257,6 +2260,9 @@ Var_Parse(const char *str, GNode *ctxt, 
 	tstr++;
 	delim = '\0';
 
+	if (*tstr == '$') {
+	    tstr = tstr2 = Var_Subst(NULL, tstr, ctxt, err);
+	}
 	while (*tstr && *tstr != endc) {
 	    char	*newStr;    /* New value to return */
 	    char	termc;	    /* Character which terminated scan */
@@ -2974,7 +2980,7 @@ Var_Parse(const char *str, GNode *ctxt, 
 			*lengthPtr = cp - start + 1;
 			VarREError(error, &pattern.re, "RE substitution error");
 			free(pattern.replace);
-			return (var_Error);
+			goto cleanup;
 		    }
 
 		    pattern.nsub = pattern.re.re_nsub + 1;
@@ -3224,6 +3230,11 @@ Var_Parse(const char *str, GNode *ctxt, 
 	free(v->name);
 	free(v);
     }
+    /*
+     * XXX: If we need to free tstr2 - tstr is  no longer valid either.
+     */
+    if (tstr2)
+	free(tstr2);
     return (nstr);
 
  bad_modifier:
@@ -3235,6 +3246,8 @@ cleanup:
     *lengthPtr = cp - start + 1;
     if (*freePtr)
 	free(nstr);
+    if (tstr2)
+	free(tstr2);
     if (delim != '\0')
 	Error("Unclosed substitution for %s (%c missing)",
 	      v->name, delim);