]> git.ipfire.org Git - thirdparty/make.git/commitdiff
Fix some bugs in variable pattern substitution (e.g. $(VAR:A=B)),
authorPaul Smith <psmith@gnu.org>
Tue, 21 Sep 2004 04:00:31 +0000 (04:00 +0000)
committerPaul Smith <psmith@gnu.org>
Tue, 21 Sep 2004 04:00:31 +0000 (04:00 +0000)
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.

ChangeLog
expand.c
function.c
job.c
read.c
tests/ChangeLog
tests/run_make_tests.pl
tests/scripts/functions/substitution
variable.h

index 759c4e423b87c2c8e9a995a185a685e32c0af3f9..9b2f30dcb82f172549ec94e8fba1909c0dd23a3f 100644 (file)
--- 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
index 2c8b4b69ba0f780ae1c65744adf8bad4b3da3a82..4440192eaeca36f68029110d75c26759eeccbad7 100644 (file)
--- 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);
                      }
index bad5258671787fdc0ded8014509b05f1584afdaf..392ff25d6b1e288a304b560c45a14a80397839b9 100644 (file)
@@ -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;
 }
-
+\f
 
 /* 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 13431d356c851a96539eac573b42db904efbfe23..c45889331ff72efc6e5e92949c0afb6ab8a9d83e 100644 (file)
--- 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 e2ad630cdd93a063ff6adfa8dd63b3e7ba218671..fc69a8e068b11aacdf57d3e9a05b36480f6a45fa 100644 (file)
--- 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);
            }
        }
index 0e84c6b0f987a732cb11a3bbd5a7bb608c45ab50..eec2ef1f7a5731d9e702418f5abae1f25fb8da98 100644 (file)
@@ -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
index 8452c6b56e9534f3be7acc8b042c6867c67e1f5b..991e7807dda3b59ab24405cd084fa3b33e7db0e1 100755 (executable)
@@ -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.
 
index 9280dbbaff1da9b2da6e8ff8018dfdbf05703023..0d2f6a2590b0ba182003eb78b78c50da3b45676c 100644 (file)
@@ -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;
 
index 613278a3c502861ee56ea4af09167d09607c7776..4a23e4ac5cda574e86e81a62c7cec17fe39a15e2 100644 (file)
@@ -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));