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

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
index 234bb0cf6e839ec3ee8f270e41a9ff9f2181a612..3cb7ce3269d384474ea2717cddf25660fa89ff77 100644 (file)
--- 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)