Re: Lack of Sanity Checking in file 'pctcl.c' for PostgreSQL 9.4.x

Michael Paquier wrote:
> On Sat, Jun 13, 2015 at 12:41 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> >> Michael Paquier wrote:
> >>> By the way, your patch does not compile properly and is not in-line
> >>> with the project's code format. See the updated patch attached ;)
> >
> >> ... or the conventions for allocating memory.  Why not just use palloc()?
> >
> > That's hardly the fault of the proposed patch.  But yeah, it seems like
> > much the best fix here is to get rid of the malloc (and strdup) calls in
> > this code in favor of using the palloc infrastructure.  Even the calls
> > that *do* have manual failure checks are not compliant with our usual
> > coding standards.
>
> Hm. Regarding the code path mentioned by Bill something like the patch
> attached is enough with a memory context for the query description.

Right.  Note this no longer needs the individual pfree() when aborting,
because the MemoryContextDelete would remove the whole thing.  (Don't
bother resubmitting; I have already fixed it.)  Will push shortly.

> Now, perhaps we could do more efforts with prodesc as well, see for
> example compile_pltcl_function for pltcl and similarly for plperl.
> Thoughts?

Right.  It would simplify the code: create a memory context child of
TopTransactionContext, then compile the function, and if successful,
then MemoryContextSetParent to some longer-lived context.  When the
function is invalidated, it's sufficient to delete the context and
create a new one.  Creating the context as child of
TopTransactionContext allows you to avoid an explicit
MemoryContextDelete() in the elog(ERROR) cases while compiling.

I do wonder how many users this code has ...

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services