From: | Magnus Hagander <magnus(at)hagander(dot)net> |
---|---|
To: | Craig Ringer <craig(at)2ndquadrant(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Refectoring of receivelog.c |
Date: | 2016-02-15 13:46:17 |
Message-ID: | CABUevEyGpbOB0uC0q-RuGc58FYNESnS8-S9pzFbjX2z+Bv+iEQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Feb 15, 2016 at 7:15 AM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
> 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 copied that lack of comments from the previous implementation :P
But yeah, I agree, it's probably a good idea to add them.
> 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.
>
It's not exactly uncommon with anonymous ones either in our code, but I see
no problem adding that.
Thanks!
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
From | Date | Subject | |
---|---|---|---|
Next Message | Teodor Sigaev | 2016-02-15 13:57:45 | Re: Small PATCH: check of 2 Perl modules |
Previous Message | Daniel Verite | 2016-02-15 13:43:44 | Re: [patch] Proposal for \crosstabview in psql |