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);