pg_basebackup is not checking IDENTIFY_SYSTEM numbre of columns

Started by Jaime Casanovaover 14 years ago4 messageshackers
Jump to latest
#1Jaime Casanova
jcasanov@systemguards.com.ec

Hi,

I just notice $SUBJECT and this could lead us to a segmentation fault
if by accident we point to a system with a different number of columns
in IDENTIFY_SYSTEM, at least i point pg_receivexlog from current head
to a 9.0 instalation and got that.

Any reason for not checking number of columns?

--
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

#2Magnus Hagander
magnus@hagander.net
In reply to: Jaime Casanova (#1)
Re: pg_basebackup is not checking IDENTIFY_SYSTEM numbre of columns

On Wed, Jan 11, 2012 at 22:31, Jaime Casanova <jaime@2ndquadrant.com> wrote:

Hi,

I just notice $SUBJECT and this could lead us to a segmentation fault
if by accident we point to a system with a different number of columns
in IDENTIFY_SYSTEM, at least i point pg_receivexlog from current head
to a 9.0 instalation and got that.

Any reason for not checking number of columns?

Hmm. I could've bet it did that. I must've taken that code out at some
point during refactoring.

No, no reason. Adding such a check would be a good idea.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#3Jaime Casanova
jcasanov@systemguards.com.ec
In reply to: Magnus Hagander (#2)
Re: pg_basebackup is not checking IDENTIFY_SYSTEM numbre of columns

On Wed, Jan 11, 2012 at 5:11 PM, Magnus Hagander <magnus@hagander.net> wrote:

No, no reason. Adding such a check would be a good idea.

ok. patch attached, it also adds a few PQclear() calls before
disconnect_and_exit().
btw, in BaseBackup() in line 1149 (after the patch is applied) there
is an exit instead of disconnect_and_exit() and that is probably a
typo too

--
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

Attachments:

basebackup_nfields.patchtext/x-patch; charset=US-ASCII; name=basebackup_nfields.patchDownload+45-14
#4Magnus Hagander
magnus@hagander.net
In reply to: Jaime Casanova (#3)
Re: pg_basebackup is not checking IDENTIFY_SYSTEM numbre of columns

On Sun, Jan 15, 2012 at 22:00, Jaime Casanova <jaime@2ndquadrant.com> wrote:

On Wed, Jan 11, 2012 at 5:11 PM, Magnus Hagander <magnus@hagander.net> wrote:

No, no reason. Adding such a check would be a good idea.

ok. patch attached, it also adds a few PQclear() calls before
disconnect_and_exit().

I don't think we need to care about all those PQclear() - it does an
exit() right after them anyway, so what's the point? (the disconnect
part is important of course, since otherwise we get a message in the
log on the server)

I've applied the patch without the PQclear()s, and changed it around
so that the error message shown is actually the same in all the
different places.

btw, in BaseBackup() in line 1149 (after the patch is applied) there
is an exit instead of disconnect_and_exit() and that is probably a
typo too

It is, indeed. Fixed.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/