Re: Incorrect initialization of sentPtr in walsender.c - Mailing list pgsql-hackers

From Simon Riggs
Subject Re: Incorrect initialization of sentPtr in walsender.c
Date
Msg-id CA+U5nM+aVaL2k0pQhu39DzEehOpo3F2vsjD1pMR91rE60i41=Q@mail.gmail.com
Whole thread Raw
In response to Re: Incorrect initialization of sentPtr in walsender.c  (Michael Paquier <michael.paquier@gmail.com>)
Responses Re: Incorrect initialization of sentPtr in walsender.c
List pgsql-hackers
On 12 September 2014 13:16, Michael Paquier <michael.paquier@gmail.com> wrote:
> On Fri, Sep 12, 2014 at 4:55 PM, Heikki Linnakangas
> <hlinnakangas@vmware.com> wrote:
>> I haven't looked at those places closely, but it seems possible that at
>> least some of those variables are supposed to be initialized to a value
>> smaller than any valid WAL position, rather than just Invalid. In other
>> words, if we defined InvalidXLogRecPtr as INT64_MAX rather than 0, we would
>> still want those variables to be initialized to zero. As I said, I didn't
>> check the code, but before we change those that ought to be checked.
>
> Ah, OK. I just had a look at that, and receivedUpto and lastComplaint
> in xlog.c need to use the lowest pointer value possible as they do a
> couple of comparisons with other positions. This is as well the case
> of sentPtr in walsender.c. However, that's not the case of writePtr
> and flushPtr in walreceiver.c as those positions are just used for
> direct comparison with LogstreamResult, so we could use
> InvalidXLogRecPtr in this case.

I don't see this patch gives us anything. All it will do is prevent
easy backpatching of related fixes.

-1 for changing the code in this kind of way

I find it confusing that the "Lowest" pointer value is also "Invalid".
Valid != Invalid

-1 for this patch

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



pgsql-hackers by date:

Previous
From: Dimitri Fontaine
Date:
Subject: Re: New Event Trigger: table_rewrite
Next
From: Simon Riggs
Date:
Subject: Re: Deferring some AtStart* allocations?