Re: Update doc changes needed
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
Import Notes
Reply to msg id not found: hpzwvmtggwt.fsf@tru64.m17n.org
OK, seems the issue is closed.
SL Baur <steve@beopen.com> writes:
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.I think this is just throwing good work after bad. The entire exercise
can be eliminated by merging the two useful fields of the 'handle'
object into PGconn (at a net cost of 4 bytes); there's no reason to do
all the bookkeeping needed to keep track of a separate handle object.I already did that, and de-exported the PQsetenv routines, earlier
today. Since I still haven't heard anyone make an argument why any
app would want to call PQsetenv, I don't see a reason to do more work
on these routines.regards, tom lane
--
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
Import Notes
Reply to msg id not found: 9550.953877665@sss.pgh.pa.us | Resolved by subject fallback