Re: BUGFIX: standby disconnect can corrupt serialized reorder buffers

On 6 January 2018 at 08:28, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services