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