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,
--
-David
david@pgmasters.net
