summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Smith <psmith@gnu.org>2004-09-21 04:00:31 +0000
committerPaul Smith <psmith@gnu.org>2004-09-21 04:00:31 +0000
commit0799ce730d2404d1cd1d03ce2f4ac07cc079c72e (patch)
tree6b9602199bec0f905f06a7a8f1c7bedb1e65b349
parent08c8105c5468ff743d2f2ff2fdf3b77a6313b53e (diff)
downloadgunmake-0799ce730d2404d1cd1d03ce2f4ac07cc079c72e.tar.gz
Fix some bugs in variable pattern substitution (e.g. $(VAR:A=B)),
reported by Markus Mauhart <qwe123@chello.at>. One was a simple typo; to fix the other we call patsubst_expand() for all instances of variable substitution, even when there is no '%'. We used to call subst_expand() with a special flag set in the latter case, but it didn't work properly in all situations. Easier to just use patsubst_expand() since that's what it is.
-rw-r--r--ChangeLog22
-rw-r--r--expand.c80
-rw-r--r--function.c81
-rw-r--r--job.c4
-rw-r--r--read.c4
-rw-r--r--tests/ChangeLog6
-rwxr-xr-xtests/run_make_tests.pl8
-rw-r--r--tests/scripts/functions/substitution43
-rw-r--r--variable.h2
9 files changed, 147 insertions, 103 deletions
diff --git a/ChangeLog b/ChangeLog
index 759c4e4..9b2f30d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,25 @@
+2004-09-20 Paul D. Smith <psmith@gnu.org>
+
+ * expand.c (variable_expand_string): Modify to invoke
+ patsubst_expand() instead of subst_expand(); the latter didn't
+ handle suffix patterns correctly.
+ * function.c (subst_expand): Remove the SUFFIX_ONLY parameter; it
+ was used only from variable_expand_string() and is no longer used
+ there.
+ (func_subst): Ditto, on call to subst_expand().
+ (patsubst_expand): Require the percent pointers to point to the
+ character after the %, not to the % itself.
+ * read.c (record_files): New call criteria for patsubst_expand().
+ * variable.h: Remove SUFFIX_ONLY from subst_expand() prototype.
+ This is to fix a bug reported by Markus Mauhart <qwe123@chello.at>.
+
+2004-09-19 Paul D. Smith <psmith@gnu.org>
+
+ * function.c (subst_expand): Fix a check in by_word: look for a
+ previous blank if we're beyond the beginning of the string, not
+ the beginning of the word.
+ Bugs reported by Markus Mauhart <qwe123@chello.at>.
+
2004-05-16 Paul D. Smith <psmith@gnu.org>
* remake.c (update_goal_chain): Change the argument specifying
diff --git a/expand.c b/expand.c
index 2c8b4b6..4440192 100644
--- a/expand.c
+++ b/expand.c
@@ -304,51 +304,49 @@ variable_expand_string (char *line, char *string, long length)
if (v == 0)
warn_undefined (beg, colon - beg);
+ /* If the variable is not empty, perform the
+ substitution. */
if (v != 0 && *v->value != '\0')
{
- char *value = (v->recursive ? recursively_expand (v)
+ char *pattern, *replace, *ppercent, *rpercent;
+ char *value = (v->recursive
+ ? recursively_expand (v)
: v->value);
- char *pattern, *percent;
- if (free_beg)
- {
- *subst_end = '\0';
- pattern = subst_beg;
- }
- else
- {
- pattern = (char *) alloca (subst_end - subst_beg
- + 1);
- bcopy (subst_beg, pattern, subst_end - subst_beg);
- pattern[subst_end - subst_beg] = '\0';
- }
- percent = find_percent (pattern);
- if (percent != 0)
- {
- char *replace;
- if (free_beg)
- {
- *replace_end = '\0';
- replace = replace_beg;
- }
- else
- {
- replace = (char *) alloca (replace_end
- - replace_beg
- + 1);
- bcopy (replace_beg, replace,
- replace_end - replace_beg);
- replace[replace_end - replace_beg] = '\0';
- }
-
- o = patsubst_expand (o, value, pattern, replace,
- percent, (char *) 0);
- }
+
+ /* Copy the pattern and the replacement. Add in an
+ extra % at the beginning to use in case there
+ isn't one in the pattern. */
+ pattern = (char *) alloca (subst_end - subst_beg + 2);
+ *(pattern++) = '%';
+ bcopy (subst_beg, pattern, subst_end - subst_beg);
+ pattern[subst_end - subst_beg] = '\0';
+
+ replace = (char *) alloca (replace_end
+ - replace_beg + 2);
+ *(replace++) = '%';
+ bcopy (replace_beg, replace,
+ replace_end - replace_beg);
+ replace[replace_end - replace_beg] = '\0';
+
+ /* Look for %. Set the percent pointers properly
+ based on whether we find one or not. */
+ ppercent = find_percent (pattern);
+ if (ppercent)
+ {
+ ++ppercent;
+ rpercent = 0;
+ }
else
- o = subst_expand (o, value,
- pattern, replace_beg,
- strlen (pattern),
- end - replace_beg,
- 0, 1);
+ {
+ ppercent = pattern;
+ rpercent = replace;
+ --pattern;
+ --replace;
+ }
+
+ o = patsubst_expand (o, value, pattern, replace,
+ ppercent, rpercent);
+
if (v->recursive)
free (value);
}
diff --git a/function.c b/function.c
index bad5258..392ff25 100644
--- a/function.c
+++ b/function.c
@@ -72,19 +72,17 @@ static struct hash_table function_table;
each occurrence of SUBST with REPLACE. TEXT is null-terminated. SLEN is
the length of SUBST and RLEN is the length of REPLACE. If BY_WORD is
nonzero, substitutions are done only on matches which are complete
- whitespace-delimited words. If SUFFIX_ONLY is nonzero, substitutions are
- done only at the ends of whitespace-delimited words. */
+ whitespace-delimited words. */
char *
subst_expand (char *o, char *text, char *subst, char *replace,
- unsigned int slen, unsigned int rlen,
- int by_word, int suffix_only)
+ unsigned int slen, unsigned int rlen, int by_word)
{
char *t = text;
unsigned int tlen = strlen (text);
char *p;
- if (slen == 0 && !by_word && !suffix_only)
+ if (slen == 0 && !by_word)
{
/* The first occurrence of "" in any string is its end. */
o = variable_buffer_output (o, t, tlen);
@@ -95,7 +93,7 @@ subst_expand (char *o, char *text, char *subst, char *replace,
do
{
- if ((by_word | suffix_only) && slen == 0)
+ if (by_word && slen == 0)
/* When matching by words, the empty string should match
the end of each word, rather than the end of the whole text. */
p = end_of_token (next_token (t));
@@ -116,11 +114,9 @@ subst_expand (char *o, char *text, char *subst, char *replace,
/* If we're substituting only by fully matched words,
or only at the ends of words, check that this case qualifies. */
- if ((by_word
- && ((p > t && !isblank ((unsigned char)p[-1]))
- || (p[slen] != '\0' && !isblank ((unsigned char)p[slen]))))
- || (suffix_only
- && (p[slen] != '\0' && !isblank ((unsigned char)p[slen]))))
+ if (by_word
+ && ((p > text && !isblank ((unsigned char)p[-1]))
+ || (p[slen] != '\0' && !isblank ((unsigned char)p[slen]))))
/* Struck out. Output the rest of the string that is
no longer to be replaced. */
o = variable_buffer_output (o, subst, slen);
@@ -138,52 +134,65 @@ subst_expand (char *o, char *text, char *subst, char *replace,
return o;
}
-
+
/* Store into VARIABLE_BUFFER at O the result of scanning TEXT
and replacing strings matching PATTERN with REPLACE.
If PATTERN_PERCENT is not nil, PATTERN has already been
run through find_percent, and PATTERN_PERCENT is the result.
If REPLACE_PERCENT is not nil, REPLACE has already been
- run through find_percent, and REPLACE_PERCENT is the result. */
+ run through find_percent, and REPLACE_PERCENT is the result.
+ Note that we expect PATTERN_PERCENT and REPLACE_PERCENT to point to the
+ character _AFTER_ the %, not to the % itself.
+*/
char *
patsubst_expand (char *o, char *text, char *pattern, char *replace,
char *pattern_percent, char *replace_percent)
{
unsigned int pattern_prepercent_len, pattern_postpercent_len;
- unsigned int replace_prepercent_len, replace_postpercent_len = 0;
+ unsigned int replace_prepercent_len, replace_postpercent_len;
char *t;
unsigned int len;
int doneany = 0;
/* We call find_percent on REPLACE before checking PATTERN so that REPLACE
will be collapsed before we call subst_expand if PATTERN has no %. */
- if (replace_percent == 0)
- replace_percent = find_percent (replace);
- if (replace_percent != 0)
+ if (!replace_percent)
+ {
+ replace_percent = find_percent (replace);
+ if (replace_percent)
+ ++replace_percent;
+ }
+
+ /* Record the length of REPLACE before and after the % so we don't have to
+ compute these lengths more than once. */
+ if (replace_percent)
{
- /* Record the length of REPLACE before and after the % so
- we don't have to compute these lengths more than once. */
- replace_prepercent_len = replace_percent - replace;
- replace_postpercent_len = strlen (replace_percent + 1);
+ replace_prepercent_len = replace_percent - replace - 1;
+ replace_postpercent_len = strlen (replace_percent);
}
else
- /* We store the length of the replacement
- so we only need to compute it once. */
- replace_prepercent_len = strlen (replace);
+ {
+ replace_prepercent_len = strlen (replace);
+ replace_postpercent_len = 0;
+ }
- if (pattern_percent == 0)
- pattern_percent = find_percent (pattern);
- if (pattern_percent == 0)
+ if (!pattern_percent)
+ {
+ pattern_percent = find_percent (pattern);
+ if (pattern_percent)
+ ++pattern_percent;
+ }
+ if (!pattern_percent)
/* With no % in the pattern, this is just a simple substitution. */
return subst_expand (o, text, pattern, replace,
- strlen (pattern), strlen (replace), 1, 0);
+ strlen (pattern), strlen (replace), 1);
/* Record the length of PATTERN before and after the %
so we don't have to compute it more than once. */
- pattern_prepercent_len = pattern_percent - pattern;
- pattern_postpercent_len = strlen (pattern_percent + 1);
+ pattern_prepercent_len = pattern_percent - pattern - 1;
+ pattern_postpercent_len = strlen (pattern_percent);
while ((t = find_next_token (&text, &len)) != 0)
{
@@ -196,16 +205,16 @@ patsubst_expand (char *o, char *text, char *pattern, char *replace,
/* Does the prefix match? */
if (!fail && pattern_prepercent_len > 0
&& (*t != *pattern
- || t[pattern_prepercent_len - 1] != pattern_percent[-1]
+ || t[pattern_prepercent_len - 1] != pattern_percent[-2]
|| !strneq (t + 1, pattern + 1, pattern_prepercent_len - 1)))
fail = 1;
/* Does the suffix match? */
if (!fail && pattern_postpercent_len > 0
- && (t[len - 1] != pattern_percent[pattern_postpercent_len]
- || t[len - pattern_postpercent_len] != pattern_percent[1]
+ && (t[len - 1] != pattern_percent[pattern_postpercent_len - 1]
+ || t[len - pattern_postpercent_len] != *pattern_percent
|| !strneq (&t[len - pattern_postpercent_len],
- &pattern_percent[1], pattern_postpercent_len - 1)))
+ pattern_percent, pattern_postpercent_len - 1)))
fail = 1;
if (fail)
@@ -226,7 +235,7 @@ patsubst_expand (char *o, char *text, char *pattern, char *replace,
len - (pattern_prepercent_len
+ pattern_postpercent_len));
/* Output the part of the replacement after the %. */
- o = variable_buffer_output (o, replace_percent + 1,
+ o = variable_buffer_output (o, replace_percent,
replace_postpercent_len);
}
}
@@ -641,7 +650,7 @@ static char *
func_subst (char *o, char **argv, const char *funcname UNUSED)
{
o = subst_expand (o, argv[2], argv[0], argv[1], strlen (argv[0]),
- strlen (argv[1]), 0, 0);
+ strlen (argv[1]), 0);
return o;
}
diff --git a/job.c b/job.c
index 13431d3..c458893 100644
--- a/job.c
+++ b/job.c
@@ -3099,8 +3099,8 @@ construct_command_argv_internal (char *line, char **restp, char *shell,
if (new_argv[0] == 0)
/* Line was empty. */
return 0;
- else
- return new_argv;
+
+ return new_argv;
slow:;
/* We must use the shell. */
diff --git a/read.c b/read.c
index e2ad630..fc69a8e 100644
--- a/read.c
+++ b/read.c
@@ -1861,7 +1861,7 @@ record_files (struct nameseq *filenames, char *pattern, char *pattern_percent,
if (percent == 0)
continue;
o = patsubst_expand (buffer, name, pattern, d->name,
- pattern_percent, percent);
+ pattern_percent+1, percent+1);
/* If the name expanded to the empty string, that's
illegal. */
if (o == buffer)
@@ -2067,7 +2067,7 @@ record_files (struct nameseq *filenames, char *pattern, char *pattern_percent,
static char *percent = "%";
char *buffer = variable_expand ("");
char *o = patsubst_expand (buffer, name, pattern, percent,
- pattern_percent, percent);
+ pattern_percent+1, percent+1);
f->stem = savestring (buffer, o - buffer);
}
}
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 0e84c6b..eec2ef1 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,9 @@
+2004-09-20 Paul D. Smith <psmith@gnu.org>
+
+ * scripts/functions/substitution: Rewrite to use run_make_test()
+ interface, and add test for substitution failures reported by
+ Markus Mauhart <qwe123@chello.at>.
+
2004-03-22 Paul D. Smith <psmith@gnu.org>
* test_driver.pl (run_each_test, toplevel, compare_output): Change
diff --git a/tests/run_make_tests.pl b/tests/run_make_tests.pl
index 8452c6b..991e780 100755
--- a/tests/run_make_tests.pl
+++ b/tests/run_make_tests.pl
@@ -64,6 +64,14 @@ sub run_make_test
$makefile = &get_tmpfile();
}
+ # If either the makestring or the answer don't end in newlines, add one In
+ # the future should we allow an option to disable this? For now if you
+ # want to test handling with no newline you have to call the underlying
+ # functions directly.
+
+ $makestring =~ /\n$/s or $makestring .= "\n";
+ $answer =~ /\n$/s or $answer .= "\n";
+
# Replace @MAKEFILE@ with the makefile name and @MAKE@ with the path to
# make in both $makestring and $answer.
diff --git a/tests/scripts/functions/substitution b/tests/scripts/functions/substitution
index 9280dbb..0d2f6a2 100644
--- a/tests/scripts/functions/substitution
+++ b/tests/scripts/functions/substitution
@@ -1,32 +1,33 @@
-$description = "The following test creates a makefile to ...";
+# -*-perl-*-
-$details = "";
-
-open(MAKEFILE,"> $makefile");
+$description = "Test the subst and patsubst functions";
-# The Contents of the MAKEFILE ...
+$details = "";
-print MAKEFILE "foo := a.o b.o c.o\n"
- ."bar := \$(foo:.o=.c)\n"
- ."bar2:= \$(foo:%.o=%.c)\n"
- ."bar3:= \$(patsubst %.c,%.o,x.c.c bar.c)\n"
- ."all:\n"
- ."\t\@echo \$(bar)\n"
- ."\t\@echo \$(bar2)\n"
- ."\t\@echo \$(bar3)\n";
+# Generic patsubst test: test both the function and variable form.
-# END of Contents of MAKEFILE
+run_make_test('
+foo := a.o b.o c.o
+bar := $(foo:.o=.c)
+bar2:= $(foo:%.o=%.c)
+bar3:= $(patsubst %.c,%.o,x.c.c bar.c)
+all:;@echo $(bar); echo $(bar2); echo $(bar3)',
+'',
+'a.c b.c c.c
+a.c b.c c.c
+x.c.o bar.o');
-close(MAKEFILE);
+# Patsubst without '%'--shouldn't match because the whole word has to match
+# in patsubst. Based on a bug report by Markus Mauhart <qwe123@chello.at>
-&run_make_with_options($makefile,"",&get_logfile);
+run_make_test('all:;@echo $(patsubst Foo,Repl,FooFoo)', '', 'FooFoo');
-# Create the answer to what should be produced by this Makefile
-$answer = "a.c b.c c.c\n"
- ."a.c b.c c.c\n"
- ."x.c.o bar.o\n";
+# Variable subst where a pattern matches multiple times in a single word.
+# Based on a bug report by Markus Mauhart <qwe123@chello.at>
-&compare_output($answer,&get_logfile(1));
+run_make_test('
+A := fooBARfooBARfoo
+all:;@echo $(A:fooBARfoo=REPL)', '', 'fooBARREPL');
1;
diff --git a/variable.h b/variable.h
index 613278a..4a23e4a 100644
--- a/variable.h
+++ b/variable.h
@@ -125,7 +125,7 @@ extern void restore_variable_buffer PARAMS ((char *buf, unsigned int len));
extern int handle_function PARAMS ((char **op, char **stringp));
extern int pattern_matches PARAMS ((char *pattern, char *percent, char *str));
extern char *subst_expand PARAMS ((char *o, char *text, char *subst, char *replace,
- unsigned int slen, unsigned int rlen, int by_word, int suffix_only));
+ unsigned int slen, unsigned int rlen, int by_word));
extern char *patsubst_expand PARAMS ((char *o, char *text, char *pattern, char *replace,
char *pattern_percent, char *replace_percent));