At Sat, 16 Apr 2016 12:50:30 +0530, Amit Kapila <amit.kapil...@gmail.com> wrote
> On Fri, Apr 15, 2016 at 11:30 AM, Kyotaro HORIGUCHI <
> horiguchi.kyot...@lab.ntt.co.jp> wrote:
> > At Fri, 15 Apr 2016 08:52:56 +0530, Amit Kapila <amit.kapil...@gmail.com>
> wrote :
> > >
> > > How about if we do all the parsing stuff in temporary context and then
> > > the results using TopMemoryContext? I don't think it will be a leak in
> > > TopMemoryContext, because next time we try to check/assign s_s_names, it
> > > will free the previous result.
> > I agree with you. A temporary context for the parser seems
> > reasonable. TopMemoryContext is created very early in main() so
> > palloc on it is effectively the same with malloc.
> > One problem is that only the top memory block is assumed to be
> > free()'d, not pfree()'d by guc_set_extra. It makes this quite
> > ugly..
> + newconfig = (SyncRepConfigData *) malloc(sizeof(SyncRepConfigData));
> Is there a reason to use malloc here, can't we use palloc directly?
The reason is the memory block is to released using free() in
guc_extra_field (not guc_set_extra). Even if we allocate and
deallocate it using palloc/pfree, the 'extra' pointer to the
block in gconf cannot be NULLed there and guc_extra_field tries
freeing it again using free() then bang.
> for both the functions SyncRepCopyConfig() and SyncRepFreeConfig(), if we
> directly use TopMemoryContext inside the function (if required) rather than
> taking it as argument, then it will simplify the code a lot.
Either is fine. I placed the parameter in order to emphasize
where the memory block is placed on, other than current memory
context nor bare heap, rather than for some practical reasons.
> +SyncRepFreeConfig(SyncRepConfigData *config, bool itself, MemoryContext
> Do we really need 'bool itself' parameter in above function?
> + if (cxt)
> + oldcxt = MemoryContextSwitchTo(cxt);
> + list_free_deep(config->members);
> + if(oldcxt)
> + MemoryContextSwitchTo(oldcxt);
> Why do you need MemoryContextSwitchTo for freeing members?
Ah, sorry. It's just a slip of my fingers.
NTT Open Source Software Center
Sent via pgsql-hackers mailing list (firstname.lastname@example.org)
To make changes to your subscription: