Обсуждение: pg_basebackup: Missing newlines in some error messages
Hi, while working on something else, I noticed that some error messages in pg_basebackup do not have a "\n" at the end, resulting in output like: |pg_basebackup: could not get COPY data stream: pg_basebackup: removing |data directory "data2" Patch attached. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.banck@credativ.de credativ GmbH, HRB Mönchengladbach 12080 USt-ID-Nummer: DE204566209 Trompeterallee 108, 41189 Mönchengladbach Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
Вложения
> On 21 Mar 2018, at 13:12, Michael Banck <michael.banck@credativ.de> wrote: > while working on something else, I noticed that some error messages in > pg_basebackup do not have a "\n" at the end, resulting in output like: > > |pg_basebackup: could not get COPY data stream: pg_basebackup: removing > |data directory “data2" There seems to be a few more in the other files, for example this (and more) in receivelog.c: - fprintf(stderr, _("%s: could not send feedback packet: %s"), + fprintf(stderr, _("%s: could not send feedback packet: %s\n"), Should they get newlines appended as well? cheers ./daniel
Daniel Gustafsson wrote: > > On 21 Mar 2018, at 13:12, Michael Banck <michael.banck@credativ.de> wrote: > > > while working on something else, I noticed that some error messages in > > pg_basebackup do not have a "\n" at the end, resulting in output like: > > > > |pg_basebackup: could not get COPY data stream: pg_basebackup: removing > > |data directory “data2" > > There seems to be a few more in the other files, for example this (and more) in > receivelog.c: > > - fprintf(stderr, _("%s: could not send feedback packet: %s"), > + fprintf(stderr, _("%s: could not send feedback packet: %s\n"), > > Should they get newlines appended as well? Note that PQerrorMessage already appends a newline, so if the %s at the end comes from that, the newline is purposely missing. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi, Am Mittwoch, den 21.03.2018, 09:46 -0300 schrieb Alvaro Herrera: > Daniel Gustafsson wrote: > > > On 21 Mar 2018, at 13:12, Michael Banck <michael.banck@credativ.de> wrote: > > > while working on something else, I noticed that some error messages in > > > pg_basebackup do not have a "\n" at the end, resulting in output like: > > > > > > > pg_basebackup: could not get COPY data stream: pg_basebackup: removing > > > > data directory “data2" > > > > There seems to be a few more in the other files, for example this (and more) in > > receivelog.c: > > > > - fprintf(stderr, _("%s: could not send feedback packet: %s"), > > + fprintf(stderr, _("%s: could not send feedback packet: %s\n"), > > > > Should they get newlines appended as well? > > Note that PQerrorMessage already appends a newline, so if the %s at the > end comes from that, the newline is purposely missing. Ah, I see, in that case my patch no longer makes sense. I apparently managed to screw up so badly that no PQerrorMessage was set, so saw the above (which indeed has no error message after the colon). Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.banck@credativ.de credativ GmbH, HRB Mönchengladbach 12080 USt-ID-Nummer: DE204566209 Trompeterallee 108, 41189 Mönchengladbach Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
Hi Michael Banck wrote: > I apparently managed to screw up so badly that no PQerrorMessage was > set, so saw the above (which indeed has no error message after the > colon). Well, maybe that's a different bug, then: maybe we should print something other than PQerrorMessage (or maybe PQerrorMessage should not return empty!). Can you reproduce the problem? -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera wrote: > Hi > > Michael Banck wrote: > > > I apparently managed to screw up so badly that no PQerrorMessage was > > set, so saw the above (which indeed has no error message after the > > colon). > > Well, maybe that's a different bug, then: maybe we should print > something other than PQerrorMessage (or maybe PQerrorMessage should not > return empty!). Can you reproduce the problem? The code does look a bit weird, /* * Get the COPY data */ res = PQgetResult(conn); if (PQresultStatus(res) != PGRES_COPY_OUT) { fprintf(stderr, _("%s: could not get COPY data stream: %s"), progname, PQerrorMessage(conn)); Note that noplace we check that there is actually an error. So maybe the conn got a result status other than COPY_OUT that's not an error ... -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi, Am Mittwoch, den 21.03.2018, 09:54 -0300 schrieb Alvaro Herrera: > Michael Banck wrote: > > I apparently managed to screw up so badly that no PQerrorMessage was > > set, so saw the above (which indeed has no error message after the > > colon). > > Well, maybe that's a different bug, then: maybe we should print > something other than PQerrorMessage (or maybe PQerrorMessage should not > return empty!). Can you reproduce the problem? It was while I was hacking on the code and mistyped something, so I think this is entirely my fault and not a valid concern. From what I could tell, I got PGRES_FATAL_ERROR back (I printed the int value, which was 7), so that could explain why there was no PQerrorMessage? Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.banck@credativ.de credativ GmbH, HRB Mönchengladbach 12080 USt-ID-Nummer: DE204566209 Trompeterallee 108, 41189 Mönchengladbach Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
Michael Banck wrote: > Hi, > > Am Mittwoch, den 21.03.2018, 09:54 -0300 schrieb Alvaro Herrera: > > Michael Banck wrote: > > > I apparently managed to screw up so badly that no PQerrorMessage was > > > set, so saw the above (which indeed has no error message after the > > > colon). > > > > Well, maybe that's a different bug, then: maybe we should print > > something other than PQerrorMessage (or maybe PQerrorMessage should not > > return empty!). Can you reproduce the problem? > > It was while I was hacking on the code and mistyped something, so I > think this is entirely my fault and not a valid concern. From what I > could tell, I got PGRES_FATAL_ERROR back (I printed the int value, which > was 7), so that could explain why there was no PQerrorMessage? If the PGresult passed to PQerrorMessage was NULL, then there's nowhere to stuff a fabricated error message either, so it all sounds good to me. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services