Can someone comment on this?


> Tom Lane <tgl@sss.pgh.pa.us> writes in pgsql-hackers@postgresql.org:
> 
> > SL Baur pointed out a few days ago that PQsetenv* are too fragile to
> > risk exporting in their current state.  I plan to make them non-exported
> > and remove 'em from the docs, unless someone comes up with a working
> > redesign PDQ.
> 
> I made a test patch last weekend if you want it.  It's been stress
> tested in an XEmacs Lisp-calling-libpq environment.
> 
> This patch adds a PQsetenvClear function that is analogous to the
> clear function for PQresult's and changes the PQsetenvPoll call to
> accept a PQsetenvHandle.  The PQsetenvHandle is freed automatically
> when called during a database connect.  When the asynchronous setenv
> calls are called by application code, it is now the responsibility of
> the application code to free the setenvHandle with PQsetenvClear.
> 
> Index: src/interfaces/libpq/fe-connect.c
> ===================================================================
> RCS file: /usr/local/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
> retrieving revision 1.123
> diff -u -r1.123 fe-connect.c
> --- src/interfaces/libpq/fe-connect.c	2000/03/11 03:08:36	1.123
> +++ src/interfaces/libpq/fe-connect.c	2000/03/24 03:38:25
> @@ -1314,6 +1314,8 @@
>  			 * variables to server.
>  			 */
>  
> +			if (conn->setenv_handle)
> +				PQsetenvClear(conn->setenv_handle);
>  			if ((conn->setenv_handle = PQsetenvStart(conn)) == NULL)
>  				goto error_return;
>  
> @@ -1327,10 +1329,12 @@
>  			   these queries. */
>  			conn->status = CONNECTION_OK;
>  
> -			switch (PQsetenvPoll(conn))
> +			switch (PQsetenvPoll(conn->setenv_handle))
>  			{
>  				case PGRES_POLLING_OK: /* Success */
>  					conn->status = CONNECTION_OK;
> +					free(conn->setenv_handle);
> +					conn->setenv_handle = (PGsetenvHandle)NULL;
>  					return PGRES_POLLING_OK;
>  
>  				case PGRES_POLLING_READING: /* Still going */
> @@ -1343,6 +1347,8 @@
>  
>  				default:
>  					conn->status = CONNECTION_SETENV;
> +					free(conn->setenv_handle);
> +					conn->setenv_handle = (PGsetenvHandle)NULL;
>  					goto error_return;
>  			}
>  			/* Unreachable */
> @@ -1385,6 +1391,11 @@
>  	if (conn == NULL || conn->status == CONNECTION_BAD)
>  		return NULL;
>  
> +	if (conn->setenv_handle) {
> +		PQsetenvClear(conn->setenv_handle);
> +		conn->setenv_handle = (PGsetenvHandle)NULL;
> +	}
> +
>  	if ((handle = malloc(sizeof(struct pg_setenv_state))) == NULL)
>  	{
>  		printfPQExpBuffer(&conn->errorMessage,
> @@ -1394,6 +1405,7 @@
>  	}
>  
>  	handle->conn = conn;
> +	conn->setenv_handle = handle;
>  	handle->res = NULL;
>  	handle->eo = EnvironmentOptions;
>  
> @@ -1416,9 +1428,10 @@
>   * ----------------
>   */
>  PostgresPollingStatusType
> -PQsetenvPoll(PGconn *conn)
> +PQsetenvPoll(PGsetenvHandle handle)
>  {
> -	PGsetenvHandle handle = conn->setenv_handle;
> +/*	PGsetenvHandle handle = conn->setenv_handle; */
> +
>  #ifdef MULTIBYTE
>  	static const char envname[] = "PGCLIENTENCODING";
>  #endif
> @@ -1503,10 +1516,10 @@
>  
>  				encoding = PQgetvalue(handle->res, 0, 0);
>  				if (!encoding)			/* this should not happen */
> -					conn->client_encoding = SQL_ASCII;
> +					handle->conn->client_encoding = SQL_ASCII;
>  				else
>  					/* set client encoding to pg_conn struct */
> -					conn->client_encoding = pg_char_to_encoding(encoding);
> +					handle->conn->client_encoding = pg_char_to_encoding(encoding);
>  				PQclear(handle->res);
>  				/* We have to keep going in order to clear up the query */
>  				goto keep_going;
> @@ -1590,7 +1603,9 @@
>  
>  		case SETENV_STATE_OK:
>  			/* Tidy up */
> -			free(handle);
> +			/* This is error prone and requires error conditions to be */
> +			/*  treated specially */
> +			/* free(handle); */
>  			return PGRES_POLLING_OK;
>  
>  		default:
> @@ -1606,7 +1621,7 @@
>  	handle->state = SETENV_STATE_FAILED; /* This may protect us even if we
>  										  * are called after the handle
>  										  * has been freed.             */
> -	free(handle);
> +	/* free(handle); */
>  	return PGRES_POLLING_FAILED;
>  }
>  
> @@ -1627,10 +1642,24 @@
>  	if (handle->state != SETENV_STATE_FAILED)
>  	{
>  		handle->state = SETENV_STATE_FAILED;
> -		free(handle);
> +		/* free(handle); */
>  	}
>  }
>  
> +/* ----------------
> + *		PQsetenvClear
> + *
> + * Explicitly release a PGsetenvHandle
> + *
> + * ----------------
> + */
> +void
> +PQsetenvClear(PGsetenvHandle handle)
> +{
> +	if (!handle) return;
> +	handle->conn->setenv_handle = (PGsetenvHandle)NULL;
> +	free(handle);
> +}
>  
>  /* ----------------
>   *		PQsetenv
> @@ -1655,6 +1684,10 @@
>  	if ((handle = PQsetenvStart(conn)) == NULL)
>  		return 0;
>  
> +	if (conn->setenv_handle)
> +		free(conn->setenv_handle);
> +	conn->setenv_handle = handle;
> +
>  	for (;;) {
>  		/*
>  		 * Wait, if necessary.  Note that the initial state (just after
> @@ -1692,7 +1725,7 @@
>  		/*
>  		 * Now try to advance the state machine.
>  		 */
> -		flag = PQsetenvPoll(conn);
> +		flag = PQsetenvPoll(handle);
>  	}
>  }
>  
> @@ -1716,6 +1749,7 @@
>  	conn->asyncStatus = PGASYNC_IDLE;
>  	conn->notifyList = DLNewList();
>  	conn->sock = -1;
> +	conn->setenv_handle = (PGsetenvHandle)NULL;
>  #ifdef USE_SSL
>  	conn->allow_ssl_try = TRUE;
>  #endif
> @@ -1868,6 +1902,9 @@
>  {
>  	if (conn)
>  	{
> +		/* It is safe to do this now */
> +		if (conn->setenv_handle)
> +			PQsetenvClear(conn->setenv_handle);
>  		closePGconn(conn);
>  		freePGconn(conn);
>  	}
> Index: src/interfaces/libpq/libpq-fe.h
> ===================================================================
> RCS file: /usr/local/cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v
> retrieving revision 1.61
> diff -u -r1.61 libpq-fe.h
> --- src/interfaces/libpq/libpq-fe.h	2000/03/11 03:08:37	1.61
> +++ src/interfaces/libpq/libpq-fe.h	2000/03/24 03:38:26
> @@ -234,8 +234,9 @@
>  	/* Passing of environment variables */
>  	/* Asynchronous (non-blocking) */
>  	extern PGsetenvHandle PQsetenvStart(PGconn *conn);
> -	extern PostgresPollingStatusType PQsetenvPoll(PGconn *conn);
> +	extern PostgresPollingStatusType PQsetenvPoll(PGsetenvHandle handle);
>  	extern void PQsetenvAbort(PGsetenvHandle handle);
> +	extern void PQsetenvClear(PGsetenvHandle handle);
>  
>  	/* Synchronous (blocking) */
>  	extern int PQsetenv(PGconn *conn);
> 


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 9 Jun 2000 22:14:10 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Update doc changes needed