Clean up some problems in SetClientEncoding: failed to honor doit flag

in all cases, leaked TopMemoryContext memory in others.  Make the
interaction between SetClientEncoding and InitializeClientEncoding
cleaner and better documented.  I suspect these changes should be
back-patched into 7.3, but will wait on Tatsuo's verification.

diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index 7b23cc80d0094180bad0b00d8fb7ba675cc55d3b..25291d34cb819ba28f74f0cf37cbb160afa071ec 100644 (file)
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/commands/variable.c,v 1.74 2003/04/25 19:45:08 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/commands/variable.c,v 1.75 2003/04/27 17:31:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -484,10 +484,10 @@ assign_client_encoding(const char *value, bool doit, bool interactive)
        return NULL;
 
    /*
-    * XXX SetClientEncoding depends on namespace functions which are not
-    * available at startup time. So we accept requested client encoding
-    * anyway which might not be valid (e.g. no conversion procs
-    * available).
+    * Note: if we are in startup phase then SetClientEncoding may not be
+    * able to really set the encoding.  In this case we will assume that
+    * the encoding is okay, and InitializeClientEncoding() will fix things
+    * once initialization is complete.
     */
    if (SetClientEncoding(encoding, doit) < 0)
    {

diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 588aefc61c4060af2d118303c6c2054971626128..a39dad6c42dfaa310fb8122b14f83898c684fd9f 100644 (file)
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -4,7 +4,7 @@
  * (currently mule internal code (mic) is used)
  * Tatsuo Ishii
  *
- * $Header: /cvsroot/pgsql/src/backend/utils/mb/mbutils.c,v 1.39 2003/03/10 22:28:18 tgl Exp $
+ * $Header: /cvsroot/pgsql/src/backend/utils/mb/mbutils.c,v 1.40 2003/04/27 17:31:25 tgl Exp $
  */
 #include "postgres.h"
 
@@ -32,108 +32,149 @@ static pg_enc2name *DatabaseEncoding = &pg_enc2name_tbl[PG_SQL_ASCII];
 static FmgrInfo *ToServerConvProc = NULL;
 static FmgrInfo *ToClientConvProc = NULL;
 
+/*
+ * During backend startup we can't set client encoding because we (a)
+ * can't look up the conversion functions, and (b) may not know the database
+ * encoding yet either.  So SetClientEncoding() just accepts anything and
+ * remembers it for InitializeClientEncoding() to apply later.
+ */
+static bool backend_startup_complete = false;
+static int pending_client_encoding = PG_SQL_ASCII;
+
+
 /* Internal functions */
 static unsigned char *perform_default_encoding_conversion(unsigned char *src,
                                    int len, bool is_client_to_server);
 static int cliplen(const unsigned char *str, int len, int limit);
 
-/* Flag to we need to initialize client encoding info */
-static bool need_to_init_client_encoding = -1;
 
 /*
  * Set the client encoding and save fmgrinfo for the conversion
- * function if necessary. if encoding conversion between client/server
- * encoding is not supported, returns -1
-*/
+ * function if necessary.  Returns 0 if okay, -1 if not (bad encoding
+ * or can't support conversion)
+ */
 int
 SetClientEncoding(int encoding, bool doit)
 {
    int         current_server_encoding;
    Oid         to_server_proc,
                to_client_proc;
-   FmgrInfo   *to_server = NULL;
-   FmgrInfo   *to_client = NULL;
+   FmgrInfo   *to_server;
+   FmgrInfo   *to_client;
    MemoryContext oldcontext;
 
-   current_server_encoding = GetDatabaseEncoding();
-
    if (!PG_VALID_FE_ENCODING(encoding))
        return (-1);
 
-   /* If we cannot actually set client encoding info, remember it
-    * so that we could set it using InitializeClientEncoding()
-    * in InitPostgres()
-    */
-   if (current_server_encoding != encoding && !IsTransactionState())
-       need_to_init_client_encoding = encoding;
-
-   if (current_server_encoding == encoding ||
-   (current_server_encoding == PG_SQL_ASCII || encoding == PG_SQL_ASCII))
+   /* Can't do anything during startup, per notes above */
+   if (!backend_startup_complete)
    {
-       ClientEncoding = &pg_enc2name_tbl[encoding];
+       if (doit)
+           pending_client_encoding = encoding;
        return 0;
    }
 
+   current_server_encoding = GetDatabaseEncoding();
+
    /*
-    * XXX We cannot use FindDefaultConversionProc() while in bootstrap or
-    * initprocessing mode since namespace functions will not work.
+    * Check for cases that require no conversion function.
     */
-   if (IsTransactionState())
+   if (current_server_encoding == encoding ||
+       (current_server_encoding == PG_SQL_ASCII ||
+        encoding == PG_SQL_ASCII))
    {
-       to_server_proc = FindDefaultConversionProc(encoding, current_server_encoding);
-       to_client_proc = FindDefaultConversionProc(current_server_encoding, encoding);
-
-       if (!OidIsValid(to_server_proc) || !OidIsValid(to_client_proc))
-           return -1;
-
-       /*
-        * load the fmgr info into TopMemoryContext so that it survives
-        * outside transaction.
-        */
-       oldcontext = MemoryContextSwitchTo(TopMemoryContext);
-       to_server = palloc(sizeof(FmgrInfo));
-       to_client = palloc(sizeof(FmgrInfo));
-       fmgr_info(to_server_proc, to_server);
-       fmgr_info(to_client_proc, to_client);
-       MemoryContextSwitchTo(oldcontext);
+       if (doit)
+       {
+           ClientEncoding = &pg_enc2name_tbl[encoding];
+
+           if (ToServerConvProc != NULL)
+           {
+               if (ToServerConvProc->fn_extra)
+                   pfree(ToServerConvProc->fn_extra);
+               pfree(ToServerConvProc);
+           }
+           ToServerConvProc = NULL;
+
+           if (ToClientConvProc != NULL)
+           {
+               if (ToClientConvProc->fn_extra)
+                   pfree(ToClientConvProc->fn_extra);
+               pfree(ToClientConvProc);
+           }
+           ToClientConvProc = NULL;
+       }
+       return 0;
    }
 
+   /*
+    * Look up the conversion functions.
+    */
+   to_server_proc = FindDefaultConversionProc(encoding,
+                                              current_server_encoding);
+   if (!OidIsValid(to_server_proc))
+       return -1;
+   to_client_proc = FindDefaultConversionProc(current_server_encoding,
+                                              encoding);
+   if (!OidIsValid(to_client_proc))
+       return -1;
+
+   /*
+    * Done if not wanting to actually apply setting.
+    */
    if (!doit)
        return 0;
 
-   if (IsTransactionState())
-   {
-       ClientEncoding = &pg_enc2name_tbl[encoding];
+   /*
+    * load the fmgr info into TopMemoryContext so that it survives
+    * outside transaction.
+    */
+   oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+   to_server = palloc(sizeof(FmgrInfo));
+   to_client = palloc(sizeof(FmgrInfo));
+   fmgr_info(to_server_proc, to_server);
+   fmgr_info(to_client_proc, to_client);
+   MemoryContextSwitchTo(oldcontext);
 
-       if (ToServerConvProc != NULL)
-       {
-           if (ToServerConvProc->fn_extra)
-               pfree(ToServerConvProc->fn_extra);
-           pfree(ToServerConvProc);
-       }
-       ToServerConvProc = to_server;
+   ClientEncoding = &pg_enc2name_tbl[encoding];
 
-       if (ToClientConvProc != NULL)
-       {
-           if (ToClientConvProc->fn_extra)
-               pfree(ToClientConvProc->fn_extra);
-           pfree(ToClientConvProc);
-       }
-       ToClientConvProc = to_client;
+   if (ToServerConvProc != NULL)
+   {
+       if (ToServerConvProc->fn_extra)
+           pfree(ToServerConvProc->fn_extra);
+       pfree(ToServerConvProc);
+   }
+   ToServerConvProc = to_server;
+
+   if (ToClientConvProc != NULL)
+   {
+       if (ToClientConvProc->fn_extra)
+           pfree(ToClientConvProc->fn_extra);
+       pfree(ToClientConvProc);
    }
+   ToClientConvProc = to_client;
+
    return 0;
 }
 
-/* Initialize client encoding if necessary.
- * called from InitPostgres() once during backend starting up.
+/*
+ * Initialize client encoding if necessary.
+ *     called from InitPostgres() once during backend starting up.
  */
 void
-InitializeClientEncoding()
+InitializeClientEncoding(void)
 {
-   if (need_to_init_client_encoding > 0)
+   Assert(!backend_startup_complete);
+   backend_startup_complete = true;
+
+   if (SetClientEncoding(pending_client_encoding, true) < 0)
    {
-       SetClientEncoding(need_to_init_client_encoding, 1);
-       need_to_init_client_encoding = -1;
+       /*
+        * Oops, the requested conversion is not available.
+        * We couldn't fail before, but we can now.
+        */
+       elog(FATAL, "Conversion between %s and %s is not supported",
+            pg_enc2name_tbl[pending_client_encoding].name,
+            GetDatabaseEncodingName());
    }
 }