Subject: Missing newlines in error messages

From: Magnus Hagander <[email-masked]>
Date: Thu, 15 Dec 2016 16:33:41 +0100

While poking around the pg_basebackup code, I noticed there are a lot of
error messa

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

---

From: Magnus Hagander <[email-masked]>
Date: Thu, 15 Dec 2016 16:35:15 +0100

(oops, accidental send button press)

While poking around the pg_basebackup code, I noticed there are a lot of
error messages are missing the \n at the end. Most of them are very
unlikely to happen, but in general I believe all error messages we print to
stderr from those binaries should have a newline at the end.

Is that wrong? :)

Second -- if we were to backpatch something like the attached patch, is
that going to cause issues for translators?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index e2875df..f0f7126 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -192,7 +192,7 @@ cleanup_directories_atexit(void)
 
 	if (made_tablespace_dirs || found_tablespace_dirs)
 		fprintf(stderr,
-				_("%s: changes to tablespace directories will not be undone"),
+				_("%s: changes to tablespace directories will not be undone\n"),
 				progname);
 }
 
@@ -1027,7 +1027,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
 	res = PQgetResult(conn);
 	if (PQresultStatus(res) != PGRES_COPY_OUT)
 	{
-		fprintf(stderr, _("%s: could not get COPY data stream: %s"),
+		fprintf(stderr, _("%s: could not get COPY data stream: %s\n"),
 				progname, PQerrorMessage(conn));
 		disconnect_and_exit(1);
 	}
@@ -1108,7 +1108,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
 		}
 		else if (r == -2)
 		{
-			fprintf(stderr, _("%s: could not read COPY data: %s"),
+			fprintf(stderr, _("%s: could not read COPY data: %s\n"),
 					progname, PQerrorMessage(conn));
 			disconnect_and_exit(1);
 		}
@@ -1295,7 +1295,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
 	res = PQgetResult(conn);
 	if (PQresultStatus(res) != PGRES_COPY_OUT)
 	{
-		fprintf(stderr, _("%s: could not get COPY data stream: %s"),
+		fprintf(stderr, _("%s: could not get COPY data stream: %s\n"),
 				progname, PQerrorMessage(conn));
 		disconnect_and_exit(1);
 	}
@@ -1324,7 +1324,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
 		}
 		else if (r == -2)
 		{
-			fprintf(stderr, _("%s: could not read COPY data: %s"),
+			fprintf(stderr, _("%s: could not read COPY data: %s\n"),
 					progname, PQerrorMessage(conn));
 			disconnect_and_exit(1);
 		}
@@ -1736,7 +1736,7 @@ BaseBackup(void)
 
 	if (PQsendQuery(conn, basebkp) == 0)
 	{
-		fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
+		fprintf(stderr, _("%s: could not send replication command \"%s\": %s\n"),
 				progname, "BASE_BACKUP", PQerrorMessage(conn));
 		disconnect_and_exit(1);
 	}
@@ -1747,7 +1747,7 @@ BaseBackup(void)
 	res = PQgetResult(conn);
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
 	{
-		fprintf(stderr, _("%s: could not initiate base backup: %s"),
+		fprintf(stderr, _("%s: could not initiate base backup: %s\n"),
 				progname, PQerrorMessage(conn));
 		disconnect_and_exit(1);
 	}
@@ -1783,7 +1783,7 @@ BaseBackup(void)
 	res = PQgetResult(conn);
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
 	{
-		fprintf(stderr, _("%s: could not get backup header: %s"),
+		fprintf(stderr, _("%s: could not get backup header: %s\n"),
 				progname, PQerrorMessage(conn));
 		disconnect_and_exit(1);
 	}
@@ -1864,7 +1864,7 @@ BaseBackup(void)
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
 	{
 		fprintf(stderr,
-		 _("%s: could not get transaction log end position from server: %s"),
+		 _("%s: could not get transaction log end position from server: %s\n"),
 				progname, PQerrorMessage(conn));
 		disconnect_and_exit(1);
 	}
@@ -1883,7 +1883,7 @@ BaseBackup(void)
 	res = PQgetResult(conn);
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 	{
-		fprintf(stderr, _("%s: final receive failed: %s"),
+		fprintf(stderr, _("%s: final receive failed: %s\n"),
 				progname, PQerrorMessage(conn));
 		disconnect_and_exit(1);
 	}
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index cb5f989..3b7e90a 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -151,7 +151,7 @@ sendFeedback(PGconn *conn, int64 now, bool force, bool replyRequested)
 
 	if (PQputCopyData(conn, replybuf, len) <= 0 || PQflush(conn))
 	{
-		fprintf(stderr, _("%s: could not send feedback packet: %s"),
+		fprintf(stderr, _("%s: could not send feedback packet: %s\n"),
 				progname, PQerrorMessage(conn));
 		return false;
 	}
@@ -261,7 +261,7 @@ StreamLogicalLog(void)
 	res = PQexec(conn, query->data);
 	if (PQresultStatus(res) != PGRES_COPY_BOTH)
 	{
-		fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
+		fprintf(stderr, _("%s: could not send replication command \"%s\": %s\n"),
 				progname, query->data, PQresultErrorMessage(res));
 		PQclear(res);
 		goto error;
@@ -366,7 +366,7 @@ StreamLogicalLog(void)
 			if (PQsocket(conn) < 0)
 			{
 				fprintf(stderr,
-						_("%s: invalid socket: %s"),
+						_("%s: invalid socket: %s\n"),
 						progname, PQerrorMessage(conn));
 				goto error;
 			}
@@ -429,7 +429,7 @@ StreamLogicalLog(void)
 			if (PQconsumeInput(conn) == 0)
 			{
 				fprintf(stderr,
-						_("%s: could not receive data from WAL stream: %s"),
+						_("%s: could not receive data from WAL stream: %s\n"),
 						progname, PQerrorMessage(conn));
 				goto error;
 			}
@@ -443,7 +443,7 @@ StreamLogicalLog(void)
 		/* Failure while reading the copy stream */
 		if (r == -2)
 		{
-			fprintf(stderr, _("%s: could not read COPY data: %s"),
+			fprintf(stderr, _("%s: could not read COPY data: %s\n"),
 					progname, PQerrorMessage(conn));
 			goto error;
 		}
@@ -563,7 +563,7 @@ StreamLogicalLog(void)
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 	{
 		fprintf(stderr,
-				_("%s: unexpected termination of replication stream: %s"),
+				_("%s: unexpected termination of replication stream: %s\n"),
 				progname, PQresultErrorMessage(res));
 		goto error;
 	}
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 4382e5d..1c6c410 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -342,7 +342,7 @@ sendFeedback(PGconn *conn, XLogRecPtr blockpos, int64 now, bool replyRequested)
 
 	if (PQputCopyData(conn, replybuf, len) <= 0 || PQflush(conn))
 	{
-		fprintf(stderr, _("%s: could not send feedback packet: %s"),
+		fprintf(stderr, _("%s: could not send feedback packet: %s\n"),
 				progname, PQerrorMessage(conn));
 		return false;
 	}
@@ -476,7 +476,7 @@ ReceiveXlogStream(PGconn *conn, StreamCtl *stream)
 		if (PQresultStatus(res) != PGRES_TUPLES_OK)
 		{
 			fprintf(stderr,
-					_("%s: could not send replication command \"%s\": %s"),
+					_("%s: could not send replication command \"%s\": %s\n"),
 					progname, "IDENTIFY_SYSTEM", PQerrorMessage(conn));
 			PQclear(res);
 			return false;
@@ -529,7 +529,7 @@ ReceiveXlogStream(PGconn *conn, StreamCtl *stream)
 			if (PQresultStatus(res) != PGRES_TUPLES_OK)
 			{
 				/* FIXME: we might send it ok, but get an error */
-				fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
+				fprintf(stderr, _("%s: could not send replication command \"%s\": %s\n"),
 					progname, "TIMELINE_HISTORY", PQresultErrorMessage(res));
 				PQclear(res);
 				return false;
@@ -569,7 +569,7 @@ ReceiveXlogStream(PGconn *conn, StreamCtl *stream)
 		res = PQexec(conn, query);
 		if (PQresultStatus(res) != PGRES_COPY_BOTH)
 		{
-			fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
+			fprintf(stderr, _("%s: could not send replication command \"%s\": %s\n"),
 					progname, "START_REPLICATION", PQresultErrorMessage(res));
 			PQclear(res);
 			return false;
@@ -633,7 +633,7 @@ ReceiveXlogStream(PGconn *conn, StreamCtl *stream)
 			if (PQresultStatus(res) != PGRES_COMMAND_OK)
 			{
 				fprintf(stderr,
-				   _("%s: unexpected termination of replication stream: %s"),
+				   _("%s: unexpected termination of replication stream: %s\n"),
 						progname, PQresultErrorMessage(res));
 				PQclear(res);
 				goto error;
@@ -671,7 +671,7 @@ ReceiveXlogStream(PGconn *conn, StreamCtl *stream)
 		{
 			/* Server returned an error. */
 			fprintf(stderr,
-					_("%s: unexpected termination of replication stream: %s"),
+					_("%s: unexpected termination of replication stream: %s\n"),
 					progname, PQresultErrorMessage(res));
 			PQclear(res);
 			goto error;
@@ -875,7 +875,7 @@ CopyStreamPoll(PGconn *conn, long timeout_ms)
 
 	if (PQsocket(conn) < 0)
 	{
-		fprintf(stderr, _("%s: invalid socket: %s"), progname,
+		fprintf(stderr, _("%s: invalid socket: %s\n"), progname,
 				PQerrorMessage(conn));
 		return -1;
 	}
@@ -947,7 +947,7 @@ CopyStreamReceive(PGconn *conn, long timeout, char **buffer)
 		if (PQconsumeInput(conn) == 0)
 		{
 			fprintf(stderr,
-					_("%s: could not receive data from WAL stream: %s"),
+					_("%s: could not receive data from WAL stream: %s\n"),
 					progname, PQerrorMessage(conn));
 			return -1;
 		}
@@ -961,7 +961,7 @@ CopyStreamReceive(PGconn *conn, long timeout, char **buffer)
 		return -2;
 	if (rawlen == -2)
 	{
-		fprintf(stderr, _("%s: could not read COPY data: %s"),
+		fprintf(stderr, _("%s: could not read COPY data: %s\n"),
 				progname, PQerrorMessage(conn));
 		return -1;
 	}
@@ -1149,7 +1149,7 @@ ProcessXLogDataMsg(PGconn *conn, StreamCtl *stream, char *copybuf, int len,
 			{
 				if (PQputCopyEnd(conn, NULL) <= 0 || PQflush(conn))
 				{
-					fprintf(stderr, _("%s: could not send copy-end packet: %s"),
+					fprintf(stderr, _("%s: could not send copy-end packet: %s\n"),
 							progname, PQerrorMessage(conn));
 					return false;
 				}
@@ -1190,7 +1190,7 @@ HandleEndOfCopyStream(PGconn *conn, StreamCtl *stream, char *copybuf,
 			if (PQputCopyEnd(conn, NULL) <= 0 || PQflush(conn))
 			{
 				fprintf(stderr,
-						_("%s: could not send copy-end packet: %s"),
+						_("%s: could not send copy-end packet: %s\n"),
 						progname, PQerrorMessage(conn));
 				PQclear(res);
 				return NULL;
@@ -1221,7 +1221,7 @@ CheckCopyStreamStop(PGconn *conn, StreamCtl *stream, XLogRecPtr blockpos,
 		}
 		if (PQputCopyEnd(conn, NULL) <= 0 || PQflush(conn))
 		{
-			fprintf(stderr, _("%s: could not send copy-end packet: %s"),
+			fprintf(stderr, _("%s: could not send copy-end packet: %s\n"),
 					progname, PQerrorMessage(conn));
 			return false;
 		}
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 595eaff..ee1090a 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -81,7 +81,7 @@ GetConnection(void)
 		conn_opts = PQconninfoParse(connection_string, &err_msg);
 		if (conn_opts == NULL)
 		{
-			fprintf(stderr, "%s: %s", progname, err_msg);
+			fprintf(stderr, "%s: %s\n", progname, err_msg);
 			exit(1);
 		}
 
@@ -192,7 +192,7 @@ GetConnection(void)
 
 	if (PQstatus(tmpconn) != CONNECTION_OK)
 	{
-		fprintf(stderr, _("%s: could not connect to server: %s"),
+		fprintf(stderr, _("%s: could not connect to server: %s\n"),
 				progname, PQerrorMessage(tmpconn));
 		PQfinish(tmpconn);
 		free(values);
@@ -260,7 +260,7 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
 	res = PQexec(conn, "IDENTIFY_SYSTEM");
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
 	{
-		fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
+		fprintf(stderr, _("%s: could not send replication command \"%s\": %s\n"),
 				progname, "IDENTIFY_SYSTEM", PQerrorMessage(conn));
 
 		PQclear(res);
@@ -364,7 +364,7 @@ CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
 		}
 		else
 		{
-			fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
+			fprintf(stderr, _("%s: could not send replication command \"%s\": %s\n"),
 					progname, query->data, PQerrorMessage(conn));
 
 			destroyPQExpBuffer(query);
@@ -410,7 +410,7 @@ DropReplicationSlot(PGconn *conn, const char *slot_name)
 	res = PQexec(conn, query->data);
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 	{
-		fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
+		fprintf(stderr, _("%s: could not send replication command \"%s\": %s\n"),
 				progname, query->data, PQerrorMessage(conn));
 
 		destroyPQExpBuffer(query);

-- 
Sent via pgsql-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

---

From: Tom Lane <[email-masked]>
Date: Thu, 15 Dec 2016 10:38:55 -0500

> Is that wrong? :)

I think you missed the fact that PQerrorMessage's result will already
end with a newline.  So most (not all) of these look OK as they stand.

			regards, tom lane

---

From: Magnus Hagander <[email-masked]>
Date: Thu, 15 Dec 2016 16:45:11 +0100

> end with a newline.  So most (not all) of these look OK as they stand.

Ha, I knew there was something. But somehow I forgot about that one. I was
wondering why nobody had found it before :)

And yeah, that leaves just the one error that I actually bumped into.  I'll
go ahead and apply that one alone then.

Pah. Less coding on jetlag.

Thanks!