Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
>> Here's an ugly attempt towards this. Though I'm unsure if we can change
>> the "const" on the PQerrorMessage parameter without messing with library
>> versions and such?
> 
> That's a bad idea in any case --- PQerrorMessage shouldn't be changing
> the state of anything.  The merging needs to happen earlier.

That was my general thought, but since I'd written it, I just threw it
out there..


>> Another option could be to change all the calls that set the error
>> message to *append* to the error message instead. Thoughts on that?
> 
> That might work ... but we'd probably have to add some "clear the
> message" calls in places.  It might be worth trying to restrict this
> convention to the connection-startup code.

How about something like this?

//Magnus
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 699,705 **** connectNoDelay(PGconn *conn)
  	{
  		char		sebuf[256];
  
! 		printfPQExpBuffer(&conn->errorMessage,
  			libpq_gettext("could not set socket to TCP no delay mode: %s\n"),
  						  SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  		return 0;
--- 699,705 ----
  	{
  		char		sebuf[256];
  
! 		appendPQExpBuffer(&conn->errorMessage,
  			libpq_gettext("could not set socket to TCP no delay mode: %s\n"),
  						  SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  		return 0;
***************
*** 729,735 **** connectFailureMessage(PGconn *conn, int errorno)
  						   NULL, 0,
  						   service, sizeof(service),
  						   NI_NUMERICSERV);
! 		printfPQExpBuffer(&conn->errorMessage,
  						  libpq_gettext("could not connect to server: %s\n"
  							"\tIs the server running locally and accepting\n"
  							"\tconnections on Unix domain socket \"%s\"?\n"),
--- 729,735 ----
  						   NULL, 0,
  						   service, sizeof(service),
  						   NI_NUMERICSERV);
! 		appendPQExpBuffer(&conn->errorMessage,
  						  libpq_gettext("could not connect to server: %s\n"
  							"\tIs the server running locally and accepting\n"
  							"\tconnections on Unix domain socket \"%s\"?\n"),
***************
*** 739,745 **** connectFailureMessage(PGconn *conn, int errorno)
  	else
  #endif   /* HAVE_UNIX_SOCKETS */
  	{
! 		printfPQExpBuffer(&conn->errorMessage,
  						  libpq_gettext("could not connect to server: %s\n"
  					 "\tIs the server running on host \"%s\" and accepting\n"
  										"\tTCP/IP connections on port %s?\n"),
--- 739,745 ----
  	else
  #endif   /* HAVE_UNIX_SOCKETS */
  	{
! 		appendPQExpBuffer(&conn->errorMessage,
  						  libpq_gettext("could not connect to server: %s\n"
  					 "\tIs the server running on host \"%s\" and accepting\n"
  										"\tTCP/IP connections on port %s?\n"),
***************
*** 829,839 **** connectDBStart(PGconn *conn)
  	if (ret || !addrs)
  	{
  		if (node)
! 			printfPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext("could not translate host name \"%s\" to address: %s\n"),
  							  node, gai_strerror(ret));
  		else
! 			printfPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext("could not translate Unix-domain socket path \"%s\" to address: %s\n"),
  							  portstr, gai_strerror(ret));
  		if (addrs)
--- 829,839 ----
  	if (ret || !addrs)
  	{
  		if (node)
! 			appendPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext("could not translate host name \"%s\" to address: %s\n"),
  							  node, gai_strerror(ret));
  		else
! 			appendPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext("could not translate Unix-domain socket path \"%s\" to address: %s\n"),
  							  portstr, gai_strerror(ret));
  		if (addrs)
***************
*** 924,929 **** connectDBComplete(PGconn *conn)
--- 924,931 ----
  		switch (flag)
  		{
  			case PGRES_POLLING_OK:
+ 				/* Reset stored error messages since we now have a working connection */
+ 				resetPQExpBuffer(&conn->errorMessage);
  				return 1;		/* success! */
  
  			case PGRES_POLLING_READING:
***************
*** 1033,1039 **** PQconnectPoll(PGconn *conn)
  			break;
  
  		default:
! 			printfPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext(
  											"invalid connection state, "
  								 "probably indicative of memory corruption\n"
--- 1035,1041 ----
  			break;
  
  		default:
! 			appendPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext(
  											"invalid connection state, "
  								 "probably indicative of memory corruption\n"
***************
*** 1077,1083 **** keep_going:						/* We will come back to here until there is
  							conn->addr_cur = addr_cur->ai_next;
  							continue;
  						}
! 						printfPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext("could not create socket: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  						break;
--- 1079,1085 ----
  							conn->addr_cur = addr_cur->ai_next;
  							continue;
  						}
! 						appendPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext("could not create socket: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  						break;
***************
*** 1100,1106 **** keep_going:						/* We will come back to here until there is
  					}
  					if (!pg_set_noblock(conn->sock))
  					{
! 						printfPQExpBuffer(&conn->errorMessage,
  										  libpq_gettext("could not set socket to non-blocking mode: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  						closesocket(conn->sock);
--- 1102,1108 ----
  					}
  					if (!pg_set_noblock(conn->sock))
  					{
! 						appendPQExpBuffer(&conn->errorMessage,
  										  libpq_gettext("could not set socket to non-blocking mode: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  						closesocket(conn->sock);
***************
*** 1112,1118 **** keep_going:						/* We will come back to here until there is
  #ifdef F_SETFD
  					if (fcntl(conn->sock, F_SETFD, FD_CLOEXEC) == -1)
  					{
! 						printfPQExpBuffer(&conn->errorMessage,
  										  libpq_gettext("could not set socket to close-on-exec mode: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  						closesocket(conn->sock);
--- 1114,1120 ----
  #ifdef F_SETFD
  					if (fcntl(conn->sock, F_SETFD, FD_CLOEXEC) == -1)
  					{
! 						appendPQExpBuffer(&conn->errorMessage,
  										  libpq_gettext("could not set socket to close-on-exec mode: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  						closesocket(conn->sock);
***************
*** 1199,1205 **** keep_going:						/* We will come back to here until there is
  				if (getsockopt(conn->sock, SOL_SOCKET, SO_ERROR,
  							   (char *) &optval, &optlen) == -1)
  				{
! 					printfPQExpBuffer(&conn->errorMessage,
  					libpq_gettext("could not get socket error status: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  					goto error_return;
--- 1201,1207 ----
  				if (getsockopt(conn->sock, SOL_SOCKET, SO_ERROR,
  							   (char *) &optval, &optlen) == -1)
  				{
! 					appendPQExpBuffer(&conn->errorMessage,
  					libpq_gettext("could not get socket error status: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  					goto error_return;
***************
*** 1237,1243 **** keep_going:						/* We will come back to here until there is
  								(struct sockaddr *) & conn->laddr.addr,
  								&conn->laddr.salen) < 0)
  				{
! 					printfPQExpBuffer(&conn->errorMessage,
  									  libpq_gettext("could not get client address from socket: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  					goto error_return;
--- 1239,1245 ----
  								(struct sockaddr *) & conn->laddr.addr,
  								&conn->laddr.salen) < 0)
  				{
! 					appendPQExpBuffer(&conn->errorMessage,
  									  libpq_gettext("could not get client address from socket: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  					goto error_return;
***************
*** 1281,1287 **** keep_going:						/* We will come back to here until there is
  					pv = htonl(NEGOTIATE_SSL_CODE);
  					if (pqPacketSend(conn, 0, &pv, sizeof(pv)) != STATUS_OK)
  					{
! 						printfPQExpBuffer(&conn->errorMessage,
  										  libpq_gettext("could not send SSL negotiation packet: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  						goto error_return;
--- 1283,1289 ----
  					pv = htonl(NEGOTIATE_SSL_CODE);
  					if (pqPacketSend(conn, 0, &pv, sizeof(pv)) != STATUS_OK)
  					{
! 						appendPQExpBuffer(&conn->errorMessage,
  										  libpq_gettext("could not send SSL negotiation packet: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  						goto error_return;
***************
*** 1303,1308 **** keep_going:						/* We will come back to here until there is
--- 1305,1311 ----
  														EnvironmentOptions);
  				if (!startpacket)
  				{
+ 					/* will not appendbuffer here, since it's likely to also run out of memory */
  					printfPQExpBuffer(&conn->errorMessage,
  									  libpq_gettext("out of memory\n"));
  					goto error_return;
***************
*** 1316,1322 **** keep_going:						/* We will come back to here until there is
  				 */
  				if (pqPacketSend(conn, 0, startpacket, packetlen) != STATUS_OK)
  				{
! 					printfPQExpBuffer(&conn->errorMessage,
  						libpq_gettext("could not send startup packet: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  					free(startpacket);
--- 1319,1325 ----
  				 */
  				if (pqPacketSend(conn, 0, startpacket, packetlen) != STATUS_OK)
  				{
! 					appendPQExpBuffer(&conn->errorMessage,
  						libpq_gettext("could not send startup packet: %s\n"),
  							SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
  					free(startpacket);
***************
*** 1381,1387 **** keep_going:						/* We will come back to here until there is
  						if (conn->sslmode[0] == 'r')	/* "require" */
  						{
  							/* Require SSL, but server does not want it */
! 							printfPQExpBuffer(&conn->errorMessage,
  											  libpq_gettext("server does not support SSL, but SSL was required\n"));
  							goto error_return;
  						}
--- 1384,1390 ----
  						if (conn->sslmode[0] == 'r')	/* "require" */
  						{
  							/* Require SSL, but server does not want it */
! 							appendPQExpBuffer(&conn->errorMessage,
  											  libpq_gettext("server does not support SSL, but SSL was required\n"));
  							goto error_return;
  						}
***************
*** 1398,1404 **** keep_going:						/* We will come back to here until there is
  						if (conn->sslmode[0] == 'r')	/* "require" */
  						{
  							/* Require SSL, but server is too old */
! 							printfPQExpBuffer(&conn->errorMessage,
  											  libpq_gettext("server does not support SSL, but SSL was required\n"));
  							goto error_return;
  						}
--- 1401,1407 ----
  						if (conn->sslmode[0] == 'r')	/* "require" */
  						{
  							/* Require SSL, but server is too old */
! 							appendPQExpBuffer(&conn->errorMessage,
  											  libpq_gettext("server does not support SSL, but SSL was required\n"));
  							goto error_return;
  						}
***************
*** 1414,1420 **** keep_going:						/* We will come back to here until there is
  					}
  					else
  					{
! 						printfPQExpBuffer(&conn->errorMessage,
  										  libpq_gettext("received invalid response to SSL negotiation: %c\n"),
  										  SSLok);
  						goto error_return;
--- 1417,1423 ----
  					}
  					else
  					{
! 						appendPQExpBuffer(&conn->errorMessage,
  										  libpq_gettext("received invalid response to SSL negotiation: %c\n"),
  										  SSLok);
  						goto error_return;
***************
*** 1489,1495 **** keep_going:						/* We will come back to here until there is
  				 */
  				if (!(beresp == 'R' || beresp == 'E'))
  				{
! 					printfPQExpBuffer(&conn->errorMessage,
  									  libpq_gettext(
  									  "expected authentication request from "
  												"server, but received %c\n"),
--- 1492,1498 ----
  				 */
  				if (!(beresp == 'R' || beresp == 'E'))
  				{
! 					appendPQExpBuffer(&conn->errorMessage,
  									  libpq_gettext(
  									  "expected authentication request from "
  												"server, but received %c\n"),
***************
*** 1522,1528 **** keep_going:						/* We will come back to here until there is
  				 */
  				if (beresp == 'R' && (msgLength < 8 || msgLength > 2000))
  				{
! 					printfPQExpBuffer(&conn->errorMessage,
  									  libpq_gettext(
  									  "expected authentication request from "
  												"server, but received %c\n"),
--- 1525,1531 ----
  				 */
  				if (beresp == 'R' && (msgLength < 8 || msgLength > 2000))
  				{
! 					appendPQExpBuffer(&conn->errorMessage,
  									  libpq_gettext(
  									  "expected authentication request from "
  												"server, but received %c\n"),
***************
*** 1534,1540 **** keep_going:						/* We will come back to here until there is
  				{
  					/* Handle error from a pre-3.0 server */
  					conn->inCursor = conn->inStart + 1; /* reread data */
! 					if (pqGets(&conn->errorMessage, conn))
  					{
  						/* We'll come back when there is more data */
  						return PGRES_POLLING_READING;
--- 1537,1543 ----
  				{
  					/* Handle error from a pre-3.0 server */
  					conn->inCursor = conn->inStart + 1; /* reread data */
! 					if (pqGets_append(&conn->errorMessage, conn))
  					{
  						/* We'll come back when there is more data */
  						return PGRES_POLLING_READING;
***************
*** 1601,1607 **** keep_going:						/* We will come back to here until there is
  					}
  					else
  					{
! 						if (pqGets(&conn->errorMessage, conn))
  						{
  							/* We'll come back when there is more data */
  							return PGRES_POLLING_READING;
--- 1604,1610 ----
  					}
  					else
  					{
! 						if (pqGets_append(&conn->errorMessage, conn))
  						{
  							/* We'll come back when there is more data */
  							return PGRES_POLLING_READING;
***************
*** 1788,1794 **** keep_going:						/* We will come back to here until there is
  				if (res)
  				{
  					if (res->resultStatus != PGRES_FATAL_ERROR)
! 						printfPQExpBuffer(&conn->errorMessage,
  										  libpq_gettext("unexpected message from server during startup\n"));
  
  					/*
--- 1791,1797 ----
  				if (res)
  				{
  					if (res->resultStatus != PGRES_FATAL_ERROR)
! 						appendPQExpBuffer(&conn->errorMessage,
  										  libpq_gettext("unexpected message from server during startup\n"));
  
  					/*
***************
*** 1855,1861 **** keep_going:						/* We will come back to here until there is
  			return PGRES_POLLING_OK;
  
  		default:
! 			printfPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext(
  											"invalid connection state %c, "
  								 "probably indicative of memory corruption\n"
--- 1858,1864 ----
  			return PGRES_POLLING_OK;
  
  		default:
! 			appendPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext(
  											"invalid connection state %c, "
  								 "probably indicative of memory corruption\n"
*** a/src/interfaces/libpq/fe-misc.c
--- b/src/interfaces/libpq/fe-misc.c
***************
*** 106,119 **** pqPutc(char c, PGconn *conn)
  
  
  /*
!  * pqGets:
   * get a null-terminated string from the connection,
   * and store it in an expansible PQExpBuffer.
   * If we run out of memory, all of the string is still read,
   * but the excess characters are silently discarded.
   */
! int
! pqGets(PQExpBuffer buf, PGconn *conn)
  {
  	/* Copy conn data to locals for faster search loop */
  	char	   *inBuffer = conn->inBuffer;
--- 106,119 ----
  
  
  /*
!  * pqGets[_append]:
   * get a null-terminated string from the connection,
   * and store it in an expansible PQExpBuffer.
   * If we run out of memory, all of the string is still read,
   * but the excess characters are silently discarded.
   */
! static int
! pqGets_internal(PQExpBuffer buf, PGconn *conn, bool resetbuffer)
  {
  	/* Copy conn data to locals for faster search loop */
  	char	   *inBuffer = conn->inBuffer;
***************
*** 129,135 **** pqGets(PQExpBuffer buf, PGconn *conn)
  
  	slen = inCursor - conn->inCursor;
  
! 	resetPQExpBuffer(buf);
  	appendBinaryPQExpBuffer(buf, inBuffer + conn->inCursor, slen);
  
  	conn->inCursor = ++inCursor;
--- 129,137 ----
  
  	slen = inCursor - conn->inCursor;
  
! 	if (resetbuffer)
! 		resetPQExpBuffer(buf);
! 
  	appendBinaryPQExpBuffer(buf, inBuffer + conn->inCursor, slen);
  
  	conn->inCursor = ++inCursor;
***************
*** 141,146 **** pqGets(PQExpBuffer buf, PGconn *conn)
--- 143,160 ----
  	return 0;
  }
  
+ int
+ pqGets(PQExpBuffer buf, PGconn *conn)
+ {
+ 	return pqGets_internal(buf, conn, true);
+ }
+ 
+ int
+ pqGets_append(PQExpBuffer buf, PGconn *conn)
+ {
+ 	return pqGets_internal(buf, conn, false);
+ }
+ 
  
  /*
   * pqPuts: write a null-terminated string to the current message
*** a/src/interfaces/libpq/fe-protocol3.c
--- b/src/interfaces/libpq/fe-protocol3.c
***************
*** 853,859 **** pqGetErrorNotice3(PGconn *conn, bool isError)
  			goto fail;
  		pqClearAsyncResult(conn);
  		conn->result = res;
- 		resetPQExpBuffer(&conn->errorMessage);
  		appendPQExpBufferStr(&conn->errorMessage, workBuf.data);
  	}
  	else
--- 853,858 ----
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
***************
*** 519,524 **** extern int	pqCheckInBufferSpace(size_t bytes_needed, PGconn *conn);
--- 519,525 ----
  extern int	pqGetc(char *result, PGconn *conn);
  extern int	pqPutc(char c, PGconn *conn);
  extern int	pqGets(PQExpBuffer buf, PGconn *conn);
+ extern int	pqGets_append(PQExpBuffer buf, PGconn *conn);
  extern int	pqPuts(const char *s, PGconn *conn);
  extern int	pqGetnchar(char *s, size_t len, PGconn *conn);
  extern int	pqPutnchar(const char *s, size_t len, PGconn *conn);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to