* Stephen Frost (sfr...@snowman.net) wrote: > The two cases in clauses.c are pretty specific and documented: > > List *subargs = list_copy(((BoolExpr *) arg)->args); > > /* overly tense code to avoid leaking unused list header */ > if (!unprocessed_args) > unprocessed_args = subargs; > else > { > List *oldhdr = unprocessed_args; > > unprocessed_args = list_concat(subargs, unprocessed_args); > pfree(oldhdr); > }
Having thought through this a bit more, with the new list implementation, it's possible to just replace the pfree(oldhdr); with list_free(oldhdr); and we may want to document or perhaps encourage that users of list_concat() consider list_free()'ing the 2nd list if memory is a concern under the new implementation. The list_copy() will allocate a new list into subargs. list_concat() with the new list implementation will just append unprocessed_args on to the end of subargs, and with unprocessed_args being replaced and that list only being referenced by oldhdr, we can go ahead and do a shallow free of that list. We may want to go back and consider other uses of list_concat() under the new list implementation, since the amount of memory leak'd now is larger than just the list header, it's the list header and the array of 8 initial list element slots. Still, these were the only places where we were worried about free'ing the list header, so perhaps the other list_concat() uses aren't in highly called areas or are in situations which get cleaned up by the memory context system sufficiently. > In transformFromClauseItem(), the pfree is: > > pfree(r_relnamespace); /* free unneeded list header */ Same here- this can be replaced with a list_free() in place of the pfree() under the new list implementation. These changes passes all regression tests, though I don't know how heavily these areas are tested in the regression suite. Thanks, Stephen
signature.asc
Description: Digital signature