Subject: Re: Replace use malloc() & friend by memory contexts for plperl and pltcl

From: Jim Nasby <[email-masked]>
Date: Mon, 7 Nov 2016 16:39:12 -0600

On 8/31/16 2:57 AM, Michael Paquier wrote:
> Hi all,
>
> Cleanup $subject has been raised a couple of times, like one year ago here:
> https://www.postgresql.org/message-id/[email-masked]
> And more recently here while working on the NULL checks for malloc():
> https://www.postgresql.org/message-id/CAB7nPqR7ozfCqc6C=2+ctCcnuehTZ-vTDQ8MMhY=[email-masked]
>
> Attached are a set of patches to replace those memory system calls by
> proper memory contexts:
> - 0001 does the cleanup work for pltcl
> - 0002 does this cleanup for plperl
>
> Thoughts?

Seems like a good idea, I'm guessing it slipped through the cracks. Do 
you want to add it to the next CF?

I've looked at the pltcl patch, and it looks sane. I did have one 
question though...

+	volatile MemoryContext plan_cxt = NULL;
...
+	MemoryContext oldcontext = CurrentMemoryContext;

Why mark one as volatile but not the other? Based on [1] ISTM there's no 
need to mark either as volatile?

1: http://www.barrgroup.com/Embedded-Systems/How-To/C-Volatile-Keyword
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461

-- 
Sent via pgsql-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

---

From: Michael Paquier <[email-masked]>
Date: Tue, 8 Nov 2016 12:48:25 +0900

On Tue, Nov 8, 2016 at 7:39 AM, Jim Nasby <[email-masked]> wrote:
> Seems like a good idea, I'm guessing it slipped through the cracks. Do you
> want to add it to the next CF?

0001 has been pushed as d062245b.

> Why mark one as volatile but not the other? Based on [1] ISTM there's no need to mark either as volatile?

plan_cxt is referenced in the PG_TRY block, and then modified in the
PG_CATCH block, so it seems to me that we had better mark it as
volatile to be POSIX-compliant. That's not the case of oldcontext.
-- 
Michael