Re: Refectoring of receivelog.c

On 15 February 2016 at 04:48, Magnus Hagander <magnus(at)hagander(dot)net> wrote:

> I was working on adding the tar streaming functionality we talked about at
> the developer meeting to pg_basebackup, and rapidly ran across the issue
> that Andres has been complaining about for a while. The code in
> receivelog.c just passes an insane number of parameters around. Adding or
> changing even a small thing ends up touching a huge number of places.
>

Other than the lack of comments on the fields in StreamCtl to indicate
their functions I think this looks good.

I didn't find any mistakes, but I do admit my eyes started glazing over
after a bit.

I'd rather not have StreamCtl as a typedef of an anonymous struct if it's
exposed in a header though. It should have a name that can be used in
forward declarations etc.

After recently working with the XLogReader I can't emphasise enough how
important *useful* comments on the fields in these sorts of structs are -
the XLogReaderState has ReadRecPtr, readSegNo + readOff + readLen,
currRecPtr AND latestPagePtr. Which actually do have comments, just not
super helpful ones.

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