Fix minor bug in regexp makesearch() function.

tglsfdc · tglsfdc · commit 91cf3135b907 · 2015-09-09T20:14:58.000-04:00
The list-wrangling here was done wrong, allowing the same state to get
put into the list twice.  The following loop then would clone it twice.
The second clone would wind up with no inarcs, so that there was no
observable misbehavior AFAICT, but a useless state in the finished NFA
isn't an especially good thing.
diff --git a/src/backend/regex/regcomp.c b/src/backend/regex/regcomp.c
@@ -568,21 +568,26 @@ makesearch(struct vars * v,
 	 * splitting each such state into progress and no-progress states.
 	 */
 
-	/* first, make a list of the states */
+	/* first, make a list of the states reachable from pre and elsewhere */
 	slist = NULL;
 	for (a = pre->outs; a != NULL; a = a->outchain)
 	{
 		s = a->to;
 		for (b = s->ins; b != NULL; b = b->inchain)
+		{
 			if (b->from != pre)
 				break;
+		}
+
+		/*
+		 * We want to mark states as being in the list already by having non
+		 * NULL tmp fields, but we can't just store the old slist value in tmp
+		 * because that doesn't work for the first such state.  Instead, the
+		 * first list entry gets its own address in tmp.
+		 */
 		if (b != NULL && s->tmp == NULL)
 		{
-			/*
-			 * Must be split if not already in the list (fixes bugs 505048,
-			 * 230589, 840258, 504785).
-			 */
-			s->tmp = slist;
+			s->tmp = (slist != NULL) ? slist : s;
 			slist = s;
 		}
 	}
@@ -601,7 +606,7 @@ makesearch(struct vars * v,
 				freearc(nfa, a);
 			}
 		}
-		s2 = s->tmp;
+		s2 = (s->tmp != s) ? s->tmp : NULL;
 		s->tmp = NULL;			/* clean up while we're at it */
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -568,21 +568,26 @@ makesearch(struct vars * v,`
`568`	`568`	`* splitting each such state into progress and no-progress states.`
`569`	`569`	`*/`
`570`	`570`
`571`		`- /* first, make a list of the states */`
	`571`	`+ /* first, make a list of the states reachable from pre and elsewhere */`
`572`	`572`	`slist = NULL;`
`573`	`573`	`for (a = pre->outs; a != NULL; a = a->outchain)`
`574`	`574`	`{`
`575`	`575`	`s = a->to;`
`576`	`576`	`for (b = s->ins; b != NULL; b = b->inchain)`
	`577`	`+ {`
`577`	`578`	`if (b->from != pre)`
`578`	`579`	`break;`
	`580`	`+ }`
	`581`	`+`
	`582`	`+ /*`
	`583`	`+ * We want to mark states as being in the list already by having non`
	`584`	`+ * NULL tmp fields, but we can't just store the old slist value in tmp`
	`585`	`+ * because that doesn't work for the first such state. Instead, the`
	`586`	`+ * first list entry gets its own address in tmp.`
	`587`	`+ */`
`579`	`588`	`if (b != NULL && s->tmp == NULL)`
`580`	`589`	`{`
`581`		`- /*`
`582`		`- * Must be split if not already in the list (fixes bugs 505048,`
`583`		`- * 230589, 840258, 504785).`
`584`		`- */`
`585`		`- s->tmp = slist;`
	`590`	`+ s->tmp = (slist != NULL) ? slist : s;`
`586`	`591`	`slist = s;`
`587`	`592`	`}`
`588`	`593`	`}`
`@@ -601,7 +606,7 @@ makesearch(struct vars * v,`
`601`	`606`	`freearc(nfa, a);`
`602`	`607`	`}`
`603`	`608`	`}`
`604`		`- s2 = s->tmp;`
	`609`	`+ s2 = (s->tmp != s) ? s->tmp : NULL;`
`605`	`610`	`s->tmp = NULL; /* clean up while we're at it */`
`606`	`611`	`}`
`607`	`612`	`}`