Hi Craig, On 1/21/18 5:45 PM, Craig Ringer wrote: > On 6 January 2018 at 08:28, Alvaro Herrera <alvherre@alvh.no-ip.org > <mailto:alvherre@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.