On 5 March 2018 at 23:25, David Steele <da...@pgmasters.net> wrote: > Hi Craig, > > On 1/21/18 5:45 PM, Craig Ringer wrote: > > On 6 January 2018 at 08:28, Alvaro Herrera <alvhe...@alvh.no-ip.org > > <mailto:alvhe...@alvh.no-ip.org>> wrote: > > > > I think this should use ReadDirExtended with an elevel less than > ERROR, > > and do nothing. > > > > Why have strcmp(.) and strcmp(..)? These are going to be skipped by > the > > comparison to "xid" prefix anyway. Looks like strcmp processing > > power waste. > > > > Please don't use bare sprintf() -- upgrade to snprintf. > > > > With this coding, if I put a root-owned file "xidfoo" in a replslot > > directory, it will PANIC the server. Is that okay? Why not read the > > file name with sscanf(), since we know the precise format it has? > Then > > we don't need to bother with random crap left around. Maybe a good > time > > to put the "xid-%u-lsn-%X-%X.snap" literal in a macro? I guess the > > rationale is that if you let random people put "xidfoo" files in your > > replication slot dirs, you deserve a PANIC anyway, but I'm not sure. > > > > I'm happy to address those comments. > > > > The PANIC probably made sense when it was only done on startup, but not > > now it's at runtime. > > > > The rest is mainly retained from the prior code, but it's a good chance > > to make those changes. > This patch was marked Waiting on Author last December. Do you know when > you'll have a chance to provide an updated patch? > > Given that it's a bug fix it would be good to see a patch for this CF, > or very soon after. > > Thanks for the reminder. I'll address it today if I can.
-- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
