From 0edcee34595a1bec86bf3bb417284a868fbf46da Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 24 Mar 2000 01:39:55 +0000
Subject: [PATCH] Fold PQsetenv working state into PGconn, rather than trying
 to maintain it in a separate object.  There's no value in keeping the state
 separate, and it creates dangling-pointer problems.  Also, remove PQsetenv
 routines from public API, until and unless they are redesigned to have a
 safer interface.  Since they were never part of the documented API before
 7.0, it's unlikely that anyone is calling them.

---
 doc/src/sgml/libpq.sgml           |  57 ---------
 src/interfaces/libpq/fe-connect.c | 205 +++++++++++-------------------
 src/interfaces/libpq/libpq-fe.h   |  17 +--
 src/interfaces/libpq/libpq-int.h  |  18 ++-
 4 files changed, 90 insertions(+), 207 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 853e8ae969f..b700ce44975 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -629,63 +629,6 @@ int PQbackendPID(const PGconn *conn);
        server host, not the local host!
       </para>
      </listitem>
-
-     <listitem>
-      <para>
-       <function>PQsetenvStart</function>
-       <function>PQsetenvPoll</function>
-       <function>PQsetenvAbort</function>
-       Perform an environment negotiation.
-       <synopsis>
-PGsetenvHandle *PQsetenvStart(PGconn *conn)
-       </synopsis>
-
-       <synopsis>
-PostgresPollingStatusType *PQsetenvPoll(PGsetenvHandle handle)
-       </synopsis>
-       <synopsis>
-void PQsetenvAbort(PGsetenvHandle handle)
-       </synopsis>
-       These two routines can be used to re-perform the environment negotiation
-       that occurs during the opening of a connection to a database
-       server. I have
-       no idea why this might be useful (XXX anyone?) but it might prove useful
-       for users to be able to reconfigure their character encodings 
-       on-the-fly, for example.
-      </para>
-      <para>
-       These functions will not block, subject to the restrictions applied to
-       PQconnectStart and PQconnectPoll.
-      </para>
-      <para>
-       To begin, call handle=PQsetenvStart(conn), where conn is an open connection
-       to the database server. If handle is NULL, then libpq has been unable to
-       allocate a new PGsetenvHandle structure. Otherwise, a valid handle is
-       returned. This handle is intended to be opaque - you may only use it to
-       call other functions in libpq (PQsetenvPoll, for example).
-      </para>
-      <para>
-       Poll the procedure using PQsetenvPoll, in exactly the same way as you would
-       create a connection using PQconnectPoll.
-      </para>
-
-      <para>
-       The procedure may be aborted at any time by calling PQsetenvAbort(handle).
-      </para>
-     </listitem>
-
-     <listitem>
-      <para>
-       <function>PQsetenv</function>
-       Perform an environment negotiation.
-       <synopsis>
-int PQsetenv(PGconn *conn)
-       </synopsis>
-       This function performs the same duties as PQsetenvStart and
-       PQsetenvPoll, but
-       blocks to do so. It returns 1 on success and 0 on failure.
-      </para>
-     </listitem>
     </itemizedlist>
    </para>
   </sect1>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index ad1d13fdf2a..df572884ebf 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v 1.124 2000/03/23 17:27:29 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v 1.125 2000/03/24 01:39:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -55,29 +55,6 @@ static int inet_aton(const char *cp, struct in_addr *inp) {
 }
 #endif
 
-/* ----------
- *     pg_setenv_state
- * A struct used when polling a setenv request. This is referred to externally
- * using a PGsetenvHandle.
- * ----------
- */
-struct pg_setenv_state
-{
-	enum
-	{
-		SETENV_STATE_OPTION_SEND,    /* About to send an Environment Option */
-		SETENV_STATE_OPTION_WAIT,    /* Waiting for above send to complete  */
-#ifdef MULTIBYTE
-		SETENV_STATE_ENCODINGS_SEND, /* About to send an "encodings" query  */
-		SETENV_STATE_ENCODINGS_WAIT, /* Waiting for query to complete       */
-#endif
-		SETENV_STATE_OK,
-		SETENV_STATE_FAILED
-	} state;
-	PGconn *conn;
-	PGresult *res;
-	const struct EnvironmentOptions *eo;
-} ;
 
 #ifdef USE_SSL
 static SSL_CTX *SSL_context = NULL;
@@ -181,6 +158,8 @@ static const struct EnvironmentOptions
 
 static int connectDBStart(PGconn *conn);
 static int connectDBComplete(PGconn *conn);
+static bool PQsetenvStart(PGconn *conn);
+static PostgresPollingStatusType PQsetenvPoll(PGconn *conn);
 static PGconn *makeEmptyPGconn(void);
 static void freePGconn(PGconn *conn);
 static void closePGconn(PGconn *conn);
@@ -1313,8 +1292,7 @@ PQconnectPoll(PGconn *conn)
 			 * Post-connection housekeeping. Prepare to send environment
 			 * variables to server.
 			 */
-
-			if ((conn->setenv_handle = PQsetenvStart(conn)) == NULL)
+			if (! PQsetenvStart(conn))
 				goto error_return;
 
 			conn->status = CONNECTION_SETENV;
@@ -1377,34 +1355,23 @@ error_return:
  *
  * ----------------
  */
-PGsetenvHandle
+static bool
 PQsetenvStart(PGconn *conn)
 {
-	struct pg_setenv_state *handle;
-
-	if (conn == NULL || conn->status == CONNECTION_BAD)
-		return NULL;
-
-	if ((handle = malloc(sizeof(struct pg_setenv_state))) == NULL)
-	{
-		printfPQExpBuffer(&conn->errorMessage,
-						  "PQsetenvStart() -- malloc error - %s\n",
-						  strerror(errno));
-		return NULL;
-	}
-
-	handle->conn = conn;
-	handle->res = NULL;
-	handle->eo = EnvironmentOptions;
+	if (conn == NULL ||
+		conn->status == CONNECTION_BAD ||
+		conn->setenv_state != SETENV_STATE_IDLE)
+		return false;
 
 #ifdef MULTIBYTE
-	handle->state = SETENV_STATE_ENCODINGS_SEND;
+	conn->setenv_state = SETENV_STATE_ENCODINGS_SEND;
 #else
-	handle->state = SETENV_STATE_OPTION_SEND;
+	conn->setenv_state = SETENV_STATE_OPTION_SEND;
 #endif
 	
-	return handle; /* Note that a struct pg_setenv_state * is the same as a
-					  PGsetenvHandle */
+	conn->next_eo = EnvironmentOptions;
+
+	return true;
 }
 
 /* ----------------
@@ -1415,19 +1382,19 @@ PQsetenvStart(PGconn *conn)
  *
  * ----------------
  */
-PostgresPollingStatusType
+static PostgresPollingStatusType
 PQsetenvPoll(PGconn *conn)
 {
-	PGsetenvHandle handle = conn->setenv_handle;
+	PGresult   *res;
 #ifdef MULTIBYTE
 	static const char envname[] = "PGCLIENTENCODING";
 #endif
 
-	if (!handle || handle->state == SETENV_STATE_FAILED)
+	if (conn == NULL || conn->status == CONNECTION_BAD)
 		return PGRES_POLLING_FAILED;
 
 	/* Check whether there are any data for us */
-	switch (handle->state)
+	switch (conn->setenv_state)
 	{
 		/* These are reading states */
 #ifdef MULTIBYTE
@@ -1436,7 +1403,7 @@ PQsetenvPoll(PGconn *conn)
 		case SETENV_STATE_OPTION_WAIT:
 		{
 			/* Load waiting data */
-			int n = pqReadData(handle->conn);
+			int n = pqReadData(conn);
 				
 			if (n < 0)
 				goto error_return;
@@ -1452,9 +1419,13 @@ PQsetenvPoll(PGconn *conn)
 #endif
 		case SETENV_STATE_OPTION_SEND:
 			break;
-		   
+
+		/* Should we raise an error if called when not active? */
+		case SETENV_STATE_IDLE:
+			return PGRES_POLLING_OK;
+
 		default:
-			printfPQExpBuffer(&handle->conn->errorMessage,
+			printfPQExpBuffer(&conn->errorMessage,
 							  "PQsetenvPoll() -- unknown state - "
 							  "probably indicative of memory corruption!\n");
 			goto error_return;
@@ -1463,7 +1434,7 @@ PQsetenvPoll(PGconn *conn)
 
  keep_going: /* We will come back to here until there is nothing left to
 				parse. */
-	switch(handle->state)
+	switch(conn->setenv_state)
 	{
 
 #ifdef MULTIBYTE
@@ -1475,39 +1446,39 @@ PQsetenvPoll(PGconn *conn)
 			env = getenv(envname);
 			if (!env || *env == '\0')
 			{
-				if (!PQsendQuery(handle->conn,
+				if (!PQsendQuery(conn,
 								 "select getdatabaseencoding()"))
 					goto error_return;
 
-				handle->state = SETENV_STATE_ENCODINGS_WAIT;
+				conn->setenv_state = SETENV_STATE_ENCODINGS_WAIT;
 				return PGRES_POLLING_READING;
 			}
 		}
 
 		case SETENV_STATE_ENCODINGS_WAIT:
 		{
-			if (PQisBusy(handle->conn))
+			if (PQisBusy(conn))
 				return PGRES_POLLING_READING;
 			
-			handle->res = PQgetResult(handle->conn);
+			res = PQgetResult(conn);
 
-			if (handle->res)
+			if (res)
 			{
 				char *encoding;
 
-				if (PQresultStatus(handle->res) != PGRES_TUPLES_OK)
+				if (PQresultStatus(res) != PGRES_TUPLES_OK)
 				{
-					PQclear(handle->res);
+					PQclear(res);
 					goto error_return;
 				}
 
-				encoding = PQgetvalue(handle->res, 0, 0);
+				/* set client encoding in pg_conn struct */
+				encoding = PQgetvalue(res, 0, 0);
 				if (!encoding)			/* this should not happen */
 					conn->client_encoding = SQL_ASCII;
 				else
-					/* set client encoding to pg_conn struct */
 					conn->client_encoding = pg_char_to_encoding(encoding);
-				PQclear(handle->res);
+				PQclear(res);
 				/* We have to keep going in order to clear up the query */
 				goto keep_going;
 			}
@@ -1515,7 +1486,7 @@ PQsetenvPoll(PGconn *conn)
 			/* NULL result indicates that the query is finished */
 
 			/* Move on to setting the environment options */
-			handle->state = SETENV_STATE_OPTION_SEND;
+			conn->setenv_state = SETENV_STATE_OPTION_SEND;
 			goto keep_going;
 		}
 #endif
@@ -1525,36 +1496,36 @@ PQsetenvPoll(PGconn *conn)
 			/* Send an Environment Option */
 			char		setQuery[100];	/* note length limits in sprintf's below */
 
-			if (handle->eo->envName)
+			if (conn->next_eo->envName)
 			{
 				const char *val;
 
-				if ((val = getenv(handle->eo->envName)))
+				if ((val = getenv(conn->next_eo->envName)))
 				{
 					if (strcasecmp(val, "default") == 0)
 						sprintf(setQuery, "SET %s = %.60s",
-								handle->eo->pgName, val);
+								conn->next_eo->pgName, val);
 					else
 						sprintf(setQuery, "SET %s = '%.60s'",
-								handle->eo->pgName, val);
+								conn->next_eo->pgName, val);
 #ifdef CONNECTDEBUG
 					printf("Use environment variable %s to send %s\n",
-						   handle->eo->envName, setQuery);
+						   conn->next_eo->envName, setQuery);
 #endif
-					if (!PQsendQuery(handle->conn, setQuery))
+					if (!PQsendQuery(conn, setQuery))
 						goto error_return;
 
-					handle->state = SETENV_STATE_OPTION_WAIT;
+					conn->setenv_state = SETENV_STATE_OPTION_WAIT;
 				}
 				else
 				{
-					handle->eo++;
+					conn->next_eo++;
 				}
 			}
 			else
 			{
-				/* No option to send, so we are done. */
-				handle->state = SETENV_STATE_OK;
+				/* No more options to send, so we are done. */
+				conn->setenv_state = SETENV_STATE_IDLE;
 			}
 
 			goto keep_going;
@@ -1562,20 +1533,20 @@ PQsetenvPoll(PGconn *conn)
 
 		case SETENV_STATE_OPTION_WAIT:
 		{
-			if (PQisBusy(handle->conn))
+			if (PQisBusy(conn))
 				return PGRES_POLLING_READING;
 			
-			handle->res = PQgetResult(handle->conn);
+			res = PQgetResult(conn);
 
-			if (handle->res)
+			if (res)
 			{
-				if (PQresultStatus(handle->res) != PGRES_COMMAND_OK)
+				if (PQresultStatus(res) != PGRES_COMMAND_OK)
 				{
-					PQclear(handle->res);
+					PQclear(res);
 					goto error_return;
 				}
 				/* Don't need the result */
-				PQclear(handle->res);
+				PQclear(res);
 				/* We have to keep going in order to clear up the query */
 				goto keep_going;
 			}
@@ -1583,18 +1554,16 @@ PQsetenvPoll(PGconn *conn)
 			/* NULL result indicates that the query is finished */
 
 			/* Send the next option */
-			handle->eo++;
-			handle->state = SETENV_STATE_OPTION_SEND;
+			conn->next_eo++;
+			conn->setenv_state = SETENV_STATE_OPTION_SEND;
 			goto keep_going;
 		}
 
-		case SETENV_STATE_OK:
-			/* Tidy up */
-			free(handle);
+		case SETENV_STATE_IDLE:
 			return PGRES_POLLING_OK;
 
 		default:
-			printfPQExpBuffer(&handle->conn->errorMessage,
+			printfPQExpBuffer(&conn->errorMessage,
 							  "PQsetenvPoll() -- unknown state - "
 							  "probably indicative of memory corruption!\n");
 			goto error_return;
@@ -1603,34 +1572,12 @@ PQsetenvPoll(PGconn *conn)
 	/* Unreachable */
 
  error_return:
-	handle->state = SETENV_STATE_FAILED; /* This may protect us even if we
-										  * are called after the handle
-										  * has been freed.             */
-	free(handle);
+	conn->setenv_state = SETENV_STATE_IDLE;
 	return PGRES_POLLING_FAILED;
 }
 
 
-/* ----------------
- *		PQsetenvAbort
- *
- * Aborts the process of passing the values of a standard set of environment
- * variables to the backend.
- *
- * ----------------
- */
-void
-PQsetenvAbort(PGsetenvHandle handle)
-{
-	/* We should not have been called in the FAILED state, but we can cope by
-	 * not freeing the handle (it has probably been freed by now anyway). */
-	if (handle->state != SETENV_STATE_FAILED)
-	{
-		handle->state = SETENV_STATE_FAILED;
-		free(handle);
-	}
-}
-
+#ifdef NOT_USED
 
 /* ----------------
  *		PQsetenv
@@ -1638,22 +1585,20 @@ PQsetenvAbort(PGsetenvHandle handle)
  * Passes the values of a standard set of environment variables to the
  * backend.
  *
- * Returns 1 on success, 0 on failure.
- *
- * This function used to return void.  I don't think that there should be
- * compatibility problems caused by giving it a return value, especially as
- * this function has not been documented previously.
+ * Returns true on success, false on failure.
  *
+ * This function used to be exported for no particularly good reason.
+ * Since it's no longer used by libpq itself, let's try #ifdef'ing it out
+ * and see if anyone complains.
  * ----------------
  */
-int
+static bool
 PQsetenv(PGconn *conn)
 {
-	PGsetenvHandle handle;
 	PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
 
-	if ((handle = PQsetenvStart(conn)) == NULL)
-		return 0;
+	if (! PQsetenvStart(conn))
+		return false;
 
 	for (;;) {
 		/*
@@ -1666,13 +1611,13 @@ PQsetenv(PGconn *conn)
 				break;
 
 			case PGRES_POLLING_OK:
-				return 1;		/* success! */
+				return true;	/* success! */
 				
 			case PGRES_POLLING_READING:
 				if (pqWait(1, 0, conn))
 				{
 					conn->status = CONNECTION_BAD;
-					return 0;
+					return false;
 				}
 				break;
 
@@ -1680,14 +1625,14 @@ PQsetenv(PGconn *conn)
 				if (pqWait(0, 1, conn))
 				{
 					conn->status = CONNECTION_BAD;
-					return 0;
+					return false;
 				}
 				break;
 
 			default:
 				/* Just in case we failed to set it in PQsetenvPoll */
 				conn->status = CONNECTION_BAD;
-				return 0;
+				return false;
 		}
 		/*
 		 * Now try to advance the state machine.
@@ -1696,6 +1641,9 @@ PQsetenv(PGconn *conn)
 	}
 }
 
+#endif /* NOT_USED */
+
+
 /*
  * makeEmptyPGconn
  *	 - create a PGconn data structure with (as yet) no interesting data
@@ -1714,6 +1662,7 @@ makeEmptyPGconn(void)
 	conn->noticeHook = defaultNoticeProcessor;
 	conn->status = CONNECTION_BAD;
 	conn->asyncStatus = PGASYNC_IDLE;
+	conn->setenv_state = SETENV_STATE_IDLE;
 	conn->notifyList = DLNewList();
 	conn->sock = -1;
 #ifdef USE_SSL
@@ -1808,12 +1757,6 @@ freePGconn(PGconn *conn)
 static void
 closePGconn(PGconn *conn)
 {
-	if (conn->status == CONNECTION_SETENV)
-	{
-		/* We have to abort the setenv process as well */
-		PQsetenvAbort(conn->setenv_handle);
-	}
-
 	if (conn->sock >= 0)
 	{
 
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 1de1592ecc3..03e81a39fc5 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: libpq-fe.h,v 1.62 2000/03/23 15:00:11 momjian Exp $
+ * $Id: libpq-fe.h,v 1.63 2000/03/24 01:39:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -86,12 +86,6 @@ extern		"C"
  */
 	typedef struct pg_result PGresult;
 
-/* PGsetenvHandle is an opaque handle which is returned by PQsetenvStart and
- * which should be passed to PQsetenvPoll or PQsetenvAbort in order to refer
- * to the particular process being performed.
- */
-	typedef struct pg_setenv_state *PGsetenvHandle;
-
 /* PGnotify represents the occurrence of a NOTIFY message.
  * Ideally this would be an opaque typedef, but it's so simple that it's
  * unlikely to change.
@@ -231,15 +225,6 @@ extern		"C"
 	/* Override default notice processor */
 	extern PQnoticeProcessor PQsetNoticeProcessor(PGconn *conn, PQnoticeProcessor proc, void *arg);
 
-	/* Passing of environment variables */
-	/* Asynchronous (non-blocking) */
-	extern PGsetenvHandle PQsetenvStart(PGconn *conn);
-	extern PostgresPollingStatusType PQsetenvPoll(PGconn *conn);
-	extern void PQsetenvAbort(PGsetenvHandle handle);
-
-	/* Synchronous (blocking) */
-	extern int PQsetenv(PGconn *conn);
-
 /* === in fe-exec.c === */
 
 	/* Simple synchronous query */
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index bb0fb8ab4a0..823abf20a33 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: libpq-int.h,v 1.21 2000/03/14 23:59:23 tgl Exp $
+ * $Id: libpq-int.h,v 1.22 2000/03/24 01:39:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -164,6 +164,17 @@ typedef enum
 	PGASYNC_COPY_OUT			/* Copy Out data transfer in progress */
 } PGAsyncStatusType;
 
+/* PGSetenvStatusType defines the state of the PQSetenv state machine */
+typedef enum
+{
+	SETENV_STATE_OPTION_SEND,	/* About to send an Environment Option */
+	SETENV_STATE_OPTION_WAIT,	/* Waiting for above send to complete  */
+	/* these next two are only used in MULTIBYTE mode */
+	SETENV_STATE_ENCODINGS_SEND, /* About to send an "encodings" query */
+	SETENV_STATE_ENCODINGS_WAIT, /* Waiting for query to complete      */
+	SETENV_STATE_IDLE
+} PGSetenvStatusType;
+
 /* large-object-access data ... allocated only if large-object code is used. */
 typedef struct pgLobjfuncs
 {
@@ -244,8 +255,9 @@ struct pg_conn
 	PGresult   *result;			/* result being constructed */
 	PGresAttValue *curTuple;	/* tuple currently being read */
 
-	/* Handle for setenv request.  Used during connection only. */
-	PGsetenvHandle setenv_handle;
+	/* Status for sending environment info.  Used during PQSetenv only. */
+	PGSetenvStatusType	setenv_state;
+	const struct EnvironmentOptions *next_eo;
 
 #ifdef USE_SSL
 	bool allow_ssl_try;			/* Allowed to try SSL negotiation */
-- 
GitLab