]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
To suppress memory leakage in long-lived Lists, lremove() should pfree
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 17 Dec 2002 01:18:35 +0000 (01:18 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 17 Dec 2002 01:18:35 +0000 (01:18 +0000)
the cons cell it's deleting from the list.  Do this, and fix a few callers
that were bogusly assuming it wouldn't free the cons cell.

src/backend/nodes/list.c
src/backend/optimizer/path/pathkeys.c
src/backend/optimizer/plan/initsplan.c
src/backend/parser/analyze.c
src/backend/rewrite/rewriteHandler.c
src/backend/utils/adt/selfuncs.c

index 6dc6001a0f209363283beee1187474eddc9dc5f5..b3c6a18496ffee05bd0b7b92f66703b26cd6b6ed 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/nodes/list.c,v 1.42 2002/11/24 21:52:13 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/nodes/list.c,v 1.43 2002/12/17 01:18:18 tgl Exp $
  *
  * NOTES
  *       XXX a few of the following functions are duplicated to handle
@@ -269,7 +269,6 @@ llasti(List *l)
  *     Free the List nodes of a list
  *     The pointed-to nodes, if any, are NOT freed.
  *     This works for integer lists too.
- *
  */
 void
 freeList(List *list)
@@ -487,6 +486,7 @@ lremove(void *elem, List *list)
                        result = lnext(l);
                else
                        lnext(prev) = lnext(l);
+               pfree(l);
        }
        return result;
 }
@@ -518,6 +518,7 @@ LispRemove(void *elem, List *list)
                        result = lnext(l);
                else
                        lnext(prev) = lnext(l);
+               pfree(l);
        }
        return result;
 }
@@ -545,6 +546,7 @@ lremovei(int elem, List *list)
                        result = lnext(l);
                else
                        lnext(prev) = lnext(l);
+               pfree(l);
        }
        return result;
 }
index af0b61a40347766139921361883f47ebc5e29b04..9c6ed4d1bd35422c2ffb69ca16f9a46f03e41eae 100644 (file)
@@ -11,7 +11,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/path/pathkeys.c,v 1.42 2002/12/12 15:49:32 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/path/pathkeys.c,v 1.43 2002/12/17 01:18:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -101,12 +101,17 @@ add_equijoined_keys(Query *root, RestrictInfo *restrictinfo)
         */
        newset = NIL;
 
-       foreach(cursetlink, root->equi_key_list)
+       /* cannot use foreach here because of possible lremove */
+       cursetlink = root->equi_key_list;
+       while (cursetlink)
        {
                List       *curset = lfirst(cursetlink);
                bool            item1here = member(item1, curset);
                bool            item2here = member(item2, curset);
 
+               /* must advance cursetlink before lremove possibly pfree's it */
+               cursetlink = lnext(cursetlink);
+
                if (item1here || item2here)
                {
                        /*
@@ -128,9 +133,7 @@ add_equijoined_keys(Query *root, RestrictInfo *restrictinfo)
                        newset = set_union(newset, curset);
 
                        /*
-                        * Remove old set from equi_key_list.  NOTE this does not
-                        * change lnext(cursetlink), so the foreach loop doesn't
-                        * break.
+                        * Remove old set from equi_key_list.
                         */
                        root->equi_key_list = lremove(curset, root->equi_key_list);
                        freeList(curset);       /* might as well recycle old cons cells */
index aca2c6f4f67cefd0405955d072bac191f17b5452..151a37a88894cc4704cc652e67d41475276bf703 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.78 2002/12/12 15:49:32 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.79 2002/12/17 01:18:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -910,13 +910,18 @@ qual_is_redundant(Query *root,
        do
        {
                someadded = false;
-               foreach(olditem, oldquals)
+               /* cannot use foreach here because of possible lremove */
+               olditem = oldquals;
+               while (olditem)
                {
                        RestrictInfo *oldrinfo = (RestrictInfo *) lfirst(olditem);
                        Node       *oldleft = (Node *) get_leftop(oldrinfo->clause);
                        Node       *oldright = (Node *) get_rightop(oldrinfo->clause);
                        Node       *newguy = NULL;
 
+                       /* must advance olditem before lremove possibly pfree's it */
+                       olditem = lnext(olditem);
+
                        if (member(oldleft, equalvars))
                                newguy = oldright;
                        else if (member(oldright, equalvars))
@@ -930,8 +935,6 @@ qual_is_redundant(Query *root,
 
                        /*
                         * Remove this qual from list, since we don't need it anymore.
-                        * Note this doesn't break the foreach() loop, since lremove
-                        * doesn't touch the next-link of the removed cons cell.
                         */
                        oldquals = lremove(oldrinfo, oldquals);
                }
index 83a322cf14874e183d73518c3f271b9a52fc8f10..e771d666597727a00acdbfc2e4e12e964ed97c45 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *     $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.257 2002/12/13 19:45:56 tgl Exp $
+ *     $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.258 2002/12/17 01:18:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -521,32 +521,38 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt,
         * Prepare columns for assignment to target table.
         */
        attnos = attrnos;
-       foreach(tl, qry->targetList)
+       /* cannot use foreach here because of possible lremove */
+       tl = qry->targetList;
+       while (tl)
        {
                TargetEntry *tle = (TargetEntry *) lfirst(tl);
                ResTarget  *col;
 
+               /* must advance tl before lremove possibly pfree's it */
+               tl = lnext(tl);
+
                if (icolumns == NIL || attnos == NIL)
                        elog(ERROR, "INSERT has more expressions than target columns");
+
                col = (ResTarget *) lfirst(icolumns);
+               Assert(IsA(col, ResTarget));
 
                /*
                 * When the value is to be set to the column default we can simply
-                * drop it now and handle it later on using methods for missing
+                * drop the TLE now and handle it later on using methods for missing
                 * columns.
                 */
-               if (!IsA(tle, InsertDefault))
+               if (IsA(tle, InsertDefault))
                {
-                       Assert(IsA(col, ResTarget));
-                       Assert(!tle->resdom->resjunk);
-                       updateTargetListEntry(pstate, tle, col->name, lfirsti(attnos),
-                                                                 col->indirection);
+                       qry->targetList = lremove(tle, qry->targetList);
+                       /* Note: the stmt->cols list is not adjusted to match */
                }
                else
                {
-                       icolumns = lremove(icolumns, icolumns);
-                       attnos = lremove(attnos, attnos);
-                       qry->targetList = lremove(tle, qry->targetList);
+                       /* Normal case */
+                       Assert(!tle->resdom->resjunk);
+                       updateTargetListEntry(pstate, tle, col->name, lfirsti(attnos),
+                                                                 col->indirection);
                }
 
                icolumns = lnext(icolumns);
@@ -554,8 +560,9 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt,
        }
 
        /*
-        * Ensure that the targetlist has the same number of  entries that
-        * were present in the columns list.  Don't do the check for select
+        * Ensure that the targetlist has the same number of entries that
+        * were present in the columns list.  Don't do the check unless
+        * an explicit columns list was given, though.
         * statements.
         */
        if (stmt->cols != NIL && (icolumns != NIL || attnos != NIL))
index d0badcc154f960f75c15db6cae898fc7d776a7be..c19d15b4f027ca98137a39c8d5d5c2670fced0f0 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.114 2002/12/12 15:49:40 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.115 2002/12/17 01:18:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -205,9 +205,11 @@ adjustJoinTreeList(Query *parsetree, bool removert, int rt_index)
                {
                        RangeTblRef *rtr = lfirst(jjt);
 
-                       if (IsA(rtr, RangeTblRef) &&rtr->rtindex == rt_index)
+                       if (IsA(rtr, RangeTblRef) &&
+                               rtr->rtindex == rt_index)
                        {
                                newjointree = lremove(rtr, newjointree);
+                               /* foreach is safe because we exit loop after lremove... */
                                break;
                        }
                }
index c378f8b50e38461247e782edb9b859408c915cdd..58c63c210cc4057ed6288a4be3b02d9fc3f5cd7f 100644 (file)
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/utils/adt/selfuncs.c,v 1.123 2002/12/12 15:49:40 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/utils/adt/selfuncs.c,v 1.124 2002/12/17 01:18:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1953,10 +1953,15 @@ estimate_num_groups(Query *root, List *groupClauses, double input_rows)
                if (HeapTupleIsValid(statsTuple))
                        ReleaseSysCache(statsTuple);
 
-               foreach(l2, varinfos)
+               /* cannot use foreach here because of possible lremove */
+               l2 = varinfos;
+               while (l2)
                {
                        MyVarInfo  *varinfo = (MyVarInfo *) lfirst(l2);
 
+                       /* must advance l2 before lremove possibly pfree's it */
+                       l2 = lnext(l2);
+
                        if (var->varno != varinfo->var->varno &&
                                vars_known_equal(root, var, varinfo->var))
                        {
@@ -1969,10 +1974,7 @@ estimate_num_groups(Query *root, List *groupClauses, double input_rows)
                                }
                                else
                                {
-                                       /*
-                                        * Delete the older item.  We assume lremove() will not
-                                        * break the lnext link of the item...
-                                        */
+                                       /* Delete the older item */
                                        varinfos = lremove(varinfo, varinfos);
                                }
                        }