Re: Re: BUGFIX: standby disconnect can corrupt serialized reorder buffers

Hi Craig,

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