Alvaro Herrera <alvhe...@2ndquadrant.com> writes: > Robbie Harwood wrote: >> Michael Paquier <michael.paqu...@gmail.com> writes: >> >> > On Tue, Apr 5, 2016 at 9:06 AM, Robbie Harwood <rharw...@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.)
Hmm, that's also an option. I read Michael's point as arguing for calloc()/free() rather than a new context, but I could be wrong. A question, though: it it valuable for the context to be reset()able separately? If there were more than just these two buffers going into it, I could see it being convenient - especially if it were for different encryption types, for instance - but it seems like it would be overkill? This is all new to me so I may be entirely mistaken though. >> 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. Hmm, that explains the Debian behavior I was seeing (it does the above). The Fedora one adds a couple blank lines in places but it's much less... gratuitous... in its changes.
Description: PGP signature