Re: Update doc changes needed

Can someone comment on this?

> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes in pgsql-hackers(at)postgresql(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026