Date: Tue, 5 Apr 2016 15:43:46 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Robbie Harwood <rharwood@redhat.com>
Cc: Michael Paquier <michael.paquier@gmail.com>,
	PostgreSQL mailing lists <pgsql-hackers@postgresql.org>,
	Magnus Hagander <magnus@hagander.net>
Subject: Re: [PATCH v12] GSSAPI encryption support
Message-ID: <20160405184346.GA317471@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <jlgmvp8rl7n.fsf@thriss.redhat.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

Robbie Harwood wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
> 
> > On Tue, Apr 5, 2016 at 9:06 AM, Robbie Harwood <rharwood@redhat.com> wrote:
> >> Here's v12, both here and on my github:
> >> https://github.com/frozencemetery/postgres/tree/feature/gssencrypt12

> > So you are saving everything in the top memory context. I am fine to
> > give the last word to a committer here but I would just go with
> > calloc/free to simplify those hunks.
> 
> Yeah, it's definitely worth thinking/talking about; this came up in IRC
> discussion as well.
> 
> If we go the memory context route, it has to be TopMemoryContext since
> nothing else lives long enough (i.e., entire connection).
[...]
> It turns out that's not actually required, but could easily be made
> explicit here.  According to the README for the memory context system,
> pfree() and repalloc() do not require setting CurrentMemoryContext
> (since 7.1).

It seems to me that the right solution for this is to create a new
memory context which is a direct child of TopMemoryContext, so that
palloc can be used, and so that it can be reset separately, and that it
doesn't suffer from resets of other contexts.  (I think Michael's point
is that if those chunks were it a context of their own, you wouldn't
need the pfrees but a MemCxtReset would be enough.)

> Side note: it's really irritating to work with having this file under
> version control because of how different it ends up being when I
> autoreconf (which I need to do because I'm changing the build system).
> (I'm also idly curious what system/autotools version is generating this
> file because it doesn't match any that I tried.)

We use stock autoconf from the GNU package and it definitely produces
matching output for me.  Maybe your Linux distro includes a patched
version?  I know Debian does, but I suppose you're using some Redhat
thing, so no idea.

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


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