From 0799ce730d2404d1cd1d03ce2f4ac07cc079c72e Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Tue, 21 Sep 2004 04:00:31 +0000 Subject: Fix some bugs in variable pattern substitution (e.g. $(VAR:A=B)), reported by Markus Mauhart . 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. --- ChangeLog | 22 ++++++++++ expand.c | 80 +++++++++++++++++------------------ function.c | 81 ++++++++++++++++++++---------------- job.c | 4 +- read.c | 4 +- tests/ChangeLog | 6 +++ tests/run_make_tests.pl | 8 ++++ tests/scripts/functions/substitution | 43 +++++++++---------- variable.h | 2 +- 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 + + * 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 . + +2004-09-19 Paul D. Smith + + * 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 . + 2004-05-16 Paul D. Smith * 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 + + * scripts/functions/substitution: Rewrite to use run_make_test() + interface, and add test for substitution failures reported by + Markus Mauhart . + 2004-03-22 Paul D. Smith * 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 -&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 -&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)); -- cgit v1.2.3