Thread: report expected contrecord size

report expected contrecord size

From
Alvaro Herrera
Date:
A pretty minor issue: when reporting that WAL appears invalid because
contrecord length doesn't match, we may as well print to the server log
the value that we're expecting.  Patch attached.

-- 
Álvaro Herrera                         http://www.flickr.com/photos/alvherre/

Attachment

Re: report expected contrecord size

From
Tom Lane
Date:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> A pretty minor issue: when reporting that WAL appears invalid because
> contrecord length doesn't match, we may as well print to the server log
> the value that we're expecting.  Patch attached.

ITYW

+                                      (long long) (total_len - gotlen),

just to be sure about what's getting casted to what.  Otherwise +1.

            regards, tom lane



Re: report expected contrecord size

From
Alvaro Herrera
Date:
On 2020-Sep-03, Tom Lane wrote:

> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > A pretty minor issue: when reporting that WAL appears invalid because
> > contrecord length doesn't match, we may as well print to the server log
> > the value that we're expecting.  Patch attached.
> 
> ITYW
> 
> +                                      (long long) (total_len - gotlen),
> 
> just to be sure about what's getting casted to what.

Well, the intention there is to cast the first operand (which is uint32)
so that it turns into signed 64-bits; the subtraction then occurs in 64
bit arithmetic normally.  If I let the subtraction occur in 32-bit width
unsigned, the result might overflow 32 bits.  I'm thinking in
1 - UINT32_MAX or some such.

Maybe to make that more explicit, it should be

+                                      ((long long) total_len) - gotlen,

(If I understand the precedence correctly, it's the same thing I wrote).

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: report expected contrecord size

From
Tom Lane
Date:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Well, the intention there is to cast the first operand (which is uint32)
> so that it turns into signed 64-bits; the subtraction then occurs in 64
> bit arithmetic normally.  If I let the subtraction occur in 32-bit width
> unsigned, the result might overflow 32 bits.

Uh ... is it really possible for gotlen to be more than total_len?
(I've not looked at the surrounding code here, but that seems weird.)

            regards, tom lane



Re: report expected contrecord size

From
Alvaro Herrera
Date:
On 2020-Sep-03, Tom Lane wrote:

> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > Well, the intention there is to cast the first operand (which is uint32)
> > so that it turns into signed 64-bits; the subtraction then occurs in 64
> > bit arithmetic normally.  If I let the subtraction occur in 32-bit width
> > unsigned, the result might overflow 32 bits.
> 
> Uh ... is it really possible for gotlen to be more than total_len?
> (I've not looked at the surrounding code here, but that seems weird.)

Well, as I understand, total_len comes from one page, and gotlen comes
from the continuation record(s) in the next page(s) of WAL.  So if
things are messed up, it could happen.  (This *is* the code that
validates the record, so it can't make too many assumptions.)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: report expected contrecord size

From
Tom Lane
Date:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2020-Sep-03, Tom Lane wrote:
>> Uh ... is it really possible for gotlen to be more than total_len?
>> (I've not looked at the surrounding code here, but that seems weird.)

> Well, as I understand, total_len comes from one page, and gotlen comes
> from the continuation record(s) in the next page(s) of WAL.  So if
> things are messed up, it could happen.  (This *is* the code that
> validates the record, so it can't make too many assumptions.)

Got it.  No further objections.

            regards, tom lane



Re: report expected contrecord size

From
Alvaro Herrera
Date:
On 2020-Sep-03, Tom Lane wrote:

> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > On 2020-Sep-03, Tom Lane wrote:
> >> Uh ... is it really possible for gotlen to be more than total_len?
> >> (I've not looked at the surrounding code here, but that seems weird.)
> 
> > Well, as I understand, total_len comes from one page, and gotlen comes
> > from the continuation record(s) in the next page(s) of WAL.  So if
> > things are messed up, it could happen.  (This *is* the code that
> > validates the record, so it can't make too many assumptions.)
> 
> Got it.  No further objections.

Many thanks for looking!  Pushed now.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services