From: jay@banzai.pcc.COM (Jay Schuster)
Subject: Make-3.62 rules bug (with fix)
Summary: singly linked list deletion code incorrect
Organization: The People's Computer Company, Williston, VT
Distribution: gnu
Date: Thu, 7 Jan 1993 16:23:44 GMT

Make-3.62 was dumping core on me with a complicated Makefile
on an IBM RS/6000 running IBM AIX 3.2.  In tracing it down,
a problem similar to that that I had with vpath was happenning
when rules were being deleted out of the list.

There was also a problem with my earlier vpath fix, in that it
referenced vpath->next when vpath may have just been free()'d.
(This was also a problem with the existing rule.c code).

This patch incorporates the vpath fix I posted yesterday, along
with a fix to it, and the fix to the rule.c.

--- ./rule.c~	Wed Jan  6 18:11:39 1993
+++ ./rule.c	Thu Jan  7 11:21:28 1993
@@ -65,13 +65,14 @@
 {
   char *name;
   unsigned int namelen;
-  register struct rule *rule, *lastrule;
+  register struct rule *rule, *lastrule, *nextrule;
 
   num_pattern_rules = 0;
   
   name = 0;
   namelen = 0;
-  rule = lastrule = pattern_rules;
+  rule = pattern_rules;
+  lastrule = NULL;
   while (rule != 0)
     {
       unsigned int ndeps = 0;
@@ -108,7 +109,8 @@
 		{
 		  if (*name == '/')
 		    {
-		      freerule (rule, lastrule);
+		      nextrule = rule->next;
+		      freerule (rule, lastrule, 1);
 		      rule = lastrule;
 		      goto end_main_loop;
 		    }
@@ -121,9 +123,11 @@
       if (ndeps > max_pattern_deps)
 	max_pattern_deps = ndeps;
 
+      nextrule = rule->next;
+
     end_main_loop:;
       lastrule = rule;
-      rule = rule->next;
+      rule = nextrule;
     }
   
   if (name != 0)
@@ -251,7 +255,7 @@
   rule->next = 0;
 
   /* Search for an identical rule.  */
-  lastrule = pattern_rules;
+  lastrule = NULL;
   for (r = pattern_rules; r != 0; lastrule = r, r = r->next)
     for (i = 0; rule->targets[i] != 0; ++i)
       for (j = 0; r->targets[j] != 0; ++j)
@@ -267,7 +271,7 @@
 	      if (override)
 		{
 		  /* Remove the old rule.  */
-		  freerule (r, lastrule);
+		  freerule (r, lastrule, 1);
 		  /* Install the new one.  */
 		  if (pattern_rules == 0)
 		    pattern_rules = rule;
@@ -281,7 +285,7 @@
 	      else
 		{
 		  /* The old rule stays intact.  Destroy the new one.  */
-		  freerule (rule, (struct rule *) 0);
+		  freerule (rule, (struct rule *) 0, 0);
 		  return 0;
 		}
 	  }
@@ -357,11 +361,14 @@
 
 /* Free all the storage used in RULE and take it out of the
    pattern_rules chain.  LASTRULE is the rule whose next pointer
-   points to RULE.  */
+   points to RULE.  DELETE should be zero to prevent the deletion
+   of RULE from the pattern_rules list (in which case LASTRULE is
+   ignored) */
 
 static void
-freerule (rule, lastrule)
+freerule (rule, lastrule, delete)
      register struct rule *rule, *lastrule;
+     int delete;
 {
   struct rule *next = rule->next;
   register unsigned int i;
@@ -386,14 +393,11 @@
 
   free ((char *) rule);
 
-  if (lastrule == 0)
+  if (!delete)
     return;
 
-  if (pattern_rules == rule)
-    if (lastrule != pattern_rules)
-      abort ();
-    else
-      pattern_rules = next;
+  if (lastrule == 0)
+    pattern_rules = next;
   else
     lastrule->next = next;
   if (last_pattern_rule == rule)
--- ./vpath.c~	Wed Jan  6 18:11:39 1993
+++ ./vpath.c	Thu Jan  7 10:57:22 1993
@@ -122,10 +122,11 @@
   if (dirpath == 0)
     {
       /* Remove matching listings.  */
-      register struct vpath *path, *lastpath;
+      register struct vpath *path, *lastpath, *nextpath;
 
-      lastpath = vpaths;
-      for (path = vpaths; path != 0; lastpath = path, path = path->next)
+      lastpath = NULL;
+      path = vpaths;
+      while (path != 0)
 	if (pattern == 0
 	    || (((percent == 0 && path->percent == 0)
 		 || (percent - pattern == path->percent - path->pattern))
@@ -132,15 +133,22 @@
 		&& streq (pattern, path->pattern)))
 	  {
 	    /* Remove it from the linked list.  */
-	    if (lastpath == vpaths)
-	      vpaths = path->next;
+	    nextpath = path->next;
+	    if (lastpath == NULL)
+	      vpaths = nextpath;
 	    else
-	      lastpath->next = path->next;
+	      lastpath->next = nextpath;
 
 	    /* Free its unused storage.  */
 	    free (path->pattern);
 	    free ((char *) path->searchpath);
 	    free ((char *) path);
+	    path = nextpath;
+	  }
+	else
+	  {
+	    lastpath = path;
+	    path = path->next;
 	  }
       if (pattern != 0)
 	free (pattern);
-- 
Jay Schuster <jay@pcc.COM>	uunet!uvm-gen!banzai!jay, attmail!banzai!jay
The People's Computer Company	`Revolutionary Programming'

Newsgroups: gnu.utils.bug