In libpq, always append new error messages to conn->errorMessage.

Previously, we had an undisciplined mish-mash of printfPQExpBuffer and
appendPQExpBuffer calls to report errors within libpq.  This commit
establishes a uniform rule that appendPQExpBuffer[Str] should be used.
conn->errorMessage is reset only at the start of an application request,
and then accumulates messages till we're done.  We can remove no less
than three different ad-hoc mechanisms that were used to get the effect
of concatenation of error messages within a sequence of operations.

Although this makes things quite a bit cleaner conceptually, the main
reason to do it is to make the world safer for the multiple-target-host
feature that was added awhile back.  Previously, there were many cases
in which an error occurring during an individual host connection attempt
would wipe out the record of what had happened during previous attempts.
(The reporting is still inadequate, in that it can be hard to tell which
host got the failure, but that seems like a matter for a separate commit.)

Currently, lo_import and lo_export contain exceptions to the "never
use printfPQExpBuffer" rule.  If we changed them, we'd risk reporting
an incidental lo_close failure before the actual read or write
failure, which would be confusing, not least because lo_close happened
after the main failure.  We could improve this by inventing an
internal version of lo_close that doesn't reset the errorMessage; but
we'd also need a version of PQfn() that does that, and it didn't quite
seem worth the trouble for now.

Discussion: https://postgr.es/m/BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com

1 files changed, 14 insertions, 14 deletions

diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c
index 8c0ba69b7c0..c783a53734e 100644
--- a/src/interfaces/libpq/fe-secure-gssapi.c
+++ b/src/interfaces/libpq/fe-secure-gssapi.c
@@ -78,7 +78,7 @@
  *
  * On success, returns the number of data bytes consumed (possibly less than
  * len).  On failure, returns -1 with errno set appropriately.  If the errno
- * indicates a non-retryable error, a message is put into conn->errorMessage.
+ * indicates a non-retryable error, a message is added to conn->errorMessage.
  * For retryable errors, caller should call again (passing the same data)
  * once the socket is ready.
  */
@@ -106,8 +106,8 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
 	 */
 	if (len < PqGSSSendConsumed)
 	{
-		printfPQExpBuffer(&conn->errorMessage,
-						  "GSSAPI caller failed to retransmit all data needing to be retried\n");
+		appendPQExpBufferStr(&conn->errorMessage,
+							 "GSSAPI caller failed to retransmit all data needing to be retried\n");
 		errno = EINVAL;
 		return -1;
 	}
@@ -205,15 +205,15 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
 
 		if (conf_state == 0)
 		{
-			printfPQExpBuffer(&conn->errorMessage,
-							  libpq_gettext("outgoing GSSAPI message would not use confidentiality\n"));
+			appendPQExpBufferStr(&conn->errorMessage,
+								 libpq_gettext("outgoing GSSAPI message would not use confidentiality\n"));
 			errno = EIO;		/* for lack of a better idea */
 			goto cleanup;
 		}
 
 		if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32))
 		{
-			printfPQExpBuffer(&conn->errorMessage,
+			appendPQExpBuffer(&conn->errorMessage,
 							  libpq_gettext("client tried to send oversize GSSAPI packet (%zu > %zu)\n"),
 							  (size_t) output.length,
 							  PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32));
@@ -258,7 +258,7 @@ cleanup:
  *
  * Returns the number of data bytes read, or on failure, returns -1
  * with errno set appropriately.  If the errno indicates a non-retryable
- * error, a message is put into conn->errorMessage.  For retryable errors,
+ * error, a message is added to conn->errorMessage.  For retryable errors,
  * caller should call again once the socket is ready.
  */
 ssize_t
@@ -350,7 +350,7 @@ pg_GSS_read(PGconn *conn, void *ptr, size_t len)
 
 		if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32))
 		{
-			printfPQExpBuffer(&conn->errorMessage,
+			appendPQExpBuffer(&conn->errorMessage,
 							  libpq_gettext("oversize GSSAPI packet sent by the server (%zu > %zu)\n"),
 							  (size_t) input.length,
 							  PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));
@@ -399,8 +399,8 @@ pg_GSS_read(PGconn *conn, void *ptr, size_t len)
 
 		if (conf_state == 0)
 		{
-			printfPQExpBuffer(&conn->errorMessage,
-							  libpq_gettext("incoming GSSAPI message did not use confidentiality\n"));
+			appendPQExpBufferStr(&conn->errorMessage,
+								 libpq_gettext("incoming GSSAPI message did not use confidentiality\n"));
 			ret = -1;
 			errno = EIO;		/* for lack of a better idea */
 			goto cleanup;
@@ -500,8 +500,8 @@ pqsecure_open_gss(PGconn *conn)
 		PqGSSResultBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE);
 		if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer)
 		{
-			printfPQExpBuffer(&conn->errorMessage,
-							  libpq_gettext("out of memory\n"));
+			appendPQExpBufferStr(&conn->errorMessage,
+								 libpq_gettext("out of memory\n"));
 			return PGRES_POLLING_FAILED;
 		}
 		PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0;
@@ -578,7 +578,7 @@ pqsecure_open_gss(PGconn *conn)
 
 			PqGSSRecvLength += ret;
 
-			printfPQExpBuffer(&conn->errorMessage, "%s\n", PqGSSRecvBuffer + 1);
+			appendPQExpBuffer(&conn->errorMessage, "%s\n", PqGSSRecvBuffer + 1);
 
 			return PGRES_POLLING_FAILED;
 		}
@@ -592,7 +592,7 @@ pqsecure_open_gss(PGconn *conn)
 		input.length = pg_ntoh32(*(uint32 *) PqGSSRecvBuffer);
 		if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32))
 		{
-			printfPQExpBuffer(&conn->errorMessage,
+			appendPQExpBuffer(&conn->errorMessage,
 							  libpq_gettext("oversize GSSAPI packet sent by the server (%zu > %zu)\n"),
 							  (size_t) input.length,
 							  PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));