report expected contrecord size

Started by Alvaro Herreraover 5 years ago7 messageshackers
Jump to latest
#1Alvaro Herrera
alvherre@2ndquadrant.com

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/

Attachments:

report-contrecord.patchtext/x-diff; charset=us-asciiDownload+2-1
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#1)
Re: report expected contrecord size

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

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#2)
Re: report expected contrecord size

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#3)
Re: report expected contrecord size

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

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#4)
Re: report expected contrecord size

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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#5)
Re: report expected contrecord size

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

#7Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#6)
Re: report expected contrecord size

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