On Thu, Apr 14, 2016 at 1:11 PM, Kyotaro HORIGUCHI
<horiguchi.kyot...@lab.ntt.co.jp> wrote:
> At Thu, 14 Apr 2016 12:42:06 +0900, Fujii Masao <masao.fu...@gmail.com> wrote 
> in <cahgqgwh7f5gwfdct71ucix_w+8ipr1owzv9k4vna1fcmyyf...@mail.gmail.com>
>> > Yes, this is what I was trying to explain to Fujii-san upthread and I have
>> > also verified that the same works on Windows.
>> Oh, okay, understood. Thanks for explaining that!
>> > I think one point which we
>> > should try to ensure in this patch is whether it is good to use
>> > TopMemoryContext to allocate the memory in the check or assign function or
>> > should we allocate some temporary context (like we do in load_tzoffsets())
>> > to perform parsing and then delete the same at end.
>> Seems yes if some memories are allocated by palloc and they are not
>> free'd while parsing s_s_names.
>> Here are another comment for the patch.
>> -SyncRepFreeConfig(SyncRepConfigData *config)
>> +SyncRepFreeConfig(SyncRepConfigData *config, bool itself)
>> SyncRepFreeConfig() was extended so that it accepts the second boolean
>> argument. But it's always called with the second argument = false. So,
>> I just wonder why that second argument is required.
>>     SyncRepConfigData *config =
>> -        (SyncRepConfigData *) palloc(sizeof(SyncRepConfigData));
>> +        (SyncRepConfigData *) malloc(sizeof(SyncRepConfigData));
>> Why should we use malloc instead of palloc here?
>> *If* we use malloc, its return value must be checked.
> Because it should live irrelevant to any memory context, as guc
> values are so. guc.c provides guc_malloc for this purpose, which
> is a malloc having some simple error handling, so having
> walsender_malloc would be reasonable.
> I don't think it's good to use TopMemoryContext for syncrep
> parser. syncrep_scanner.l uses palloc. This basically causes a
> memory leak on all postgres processes.
> It might be better if the parser works on the current memory
> context and the caller copies the result on the malloc'ed
> memory. But some list-creation functions using palloc.. Changing
> SyncRepConfigData.members to be char** would be messier..

SyncRepGetSyncStandby logic assumes deeply that the sync standby names
are constructed as a list.
I think that it would entail a radical change in SyncRepGetStandby
Another idea is to prepare the some functions that allocate/free
element of list using by malloc, free.


Masahiko Sawada

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

Reply via email to