Thread: pg_basebackup is not checking IDENTIFY_SYSTEM numbre of columns
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
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/
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
Attachment
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/