pg_basebackup: Missing newlines in some error messages

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