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. Regards, -- Masahiko Sawada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
