* Stephen Frost (sfrost@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
