foreach() and list_delete() don't mix.

tglsfdc · tglsfdc · commit 88dc6fa7a164 · 2011-04-17T13:37:39.000-04:00
Fix crash when releasing duplicate entries in the encoding conversion cache
list, caused by releasing the current entry of the list being chased by
foreach().  We have a standard idiom for handling such cases, but this
loop wasn't using it.

This got broken in my recent rewrite of GUC assign hooks.  Not sure how
I missed this when testing the modified code, but I did.  Per report from
Peter.
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
@@ -189,6 +189,8 @@ SetClientEncoding(int encoding)
 	int			current_server_encoding;
 	bool		found;
 	ListCell   *lc;
+	ListCell   *prev;
+	ListCell   *next;
 
 	if (!PG_VALID_FE_ENCODING(encoding))
 		return -1;
@@ -222,10 +224,13 @@ SetClientEncoding(int encoding)
 	 * leak memory.
 	 */
 	found = false;
-	foreach(lc, ConvProcList)
+	prev = NULL;
+	for (lc = list_head(ConvProcList); lc; lc = next)
 	{
 		ConvProcInfo *convinfo = (ConvProcInfo *) lfirst(lc);
 
+		next = lnext(lc);
+
 		if (convinfo->s_encoding == current_server_encoding &&
 			convinfo->c_encoding == encoding)
 		{
@@ -240,10 +245,13 @@ SetClientEncoding(int encoding)
 			else
 			{
 				/* Duplicate entry, release it */
-				ConvProcList = list_delete_ptr(ConvProcList, convinfo);
+				ConvProcList = list_delete_cell(ConvProcList, lc, prev);
 				pfree(convinfo);
+				continue;		/* prev mustn't advance */
 			}
 		}
+
+		prev = lc;
 	}
 
 	if (found)
